[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] qemu-img: add seek and -n option to dd command
From: |
Eric Blake |
Subject: |
Re: [PATCH] qemu-img: add seek and -n option to dd command |
Date: |
Thu, 4 Feb 2021 14:44:03 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 |
On 2/4/21 2:09 PM, Peter Lieven wrote:
> Am 02.02.21 um 16:51 schrieb Eric Blake:
>> On 1/28/21 8:07 AM, Peter Lieven wrote:
>>> Signed-off-by: Peter Lieven <pl@kamp.de>
>> Your commit message says 'what', but not 'why'. Generally, the one-line
>> 'what' works well as the subject line, but you want the commit body to
>> give an argument why your patch should be applied, rather than blank.
>>
>> Here's the last time we tried to improve qemu-img dd:
>> https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg02618.html
>
>
> I was not aware of that story. My use case is that I want to be
>
> able to "patch" an image that Qemu is able to handle by overwriting
>
> certain sectors. And I especially do not want to "mount" that image
>
> via qemu-nbd because I might not trust it. I totally want to avoid that the
> host
>
> system tries to analyse that image in terms of scanning the bootsector,
> partprobe,
>
> lvm etc. pp.
qemu-nbd does not have to mount an image (yes, one use of qemu-nbd is to
use -c /dev/nbdX to get the kernel to mount it; but other uses are to
expose the NBD image in user-space only with no kernel involvement, and
therefore no system mount efforts).
Another thing you might try is libnbd, which now includes a utility
nbdcopy. It should make it easier to overwrite a portion of an NBD
image using only user-space actions. I'm not sure if Rich has got it
doing partial file overwrites yet (.../me goes and compiles the latest
git checkout... nope, still a TODO item to implement subsetting), but it
may be possible to combine nbdkit's --filter=offset with the full NBD
image in order to then easily point nbdcopy to only the subset you care
about. Definitely some ideas worthy of implementation.
>
>>
>> where I also proposed adding seek=, and fixing skip= with count=. Your
>> patch does not do the latter. But the bigger complaint back then was
>> that 'qemu-img copy' should be able to do everything, and that qemu-img
>> dd should then just be a thin shim around 'qemu-img copy', rather than
>> having two parallel projects that diverge in their implementations.
>
>
> understood. I was not aware of an issue with skip and count.
>
> The patch works for me and I wanted to share it. But when I read
>
> the thread it seems that it would be a difficult task to get it merged.
Just because we're reluctant to improve qemu-img dd without also
improving qemu-img copy does not mean that your improvements are
unwanted. And hacking on nbdcopy may be faster than waiting for
qemu-img to catch up to where we want it to go.
>
>
>>
>> Your patch does not have the typical '---' divider and diffstat between
>> the commit message and the patch proper; this may be a factor of which
>> git packages you have installed, but having the diffstat present makes
>> it easier to see at a glance what your patch touches without reading the
>> entire email. I had to go hunting to learn if you added iotest coverage
>> of this new feature...
>>
>> ...and the answer was no, you didn't. You'll need to add that in v2
>> (see the link to my earlier attempt at modifying dd for an example).
>
>
> I did not. Maybe I accidently killed the '---' divider. If I will make a V2 I
> will add
>
> an I/O test.
>
>
> Thanks for your suggestions,
Good luck!
>
> Peter
>
>
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- Re: [PATCH] qemu-img: add seek and -n option to dd command, David Edmondson, 2021/02/02
- Re: [PATCH] qemu-img: add seek and -n option to dd command, Eric Blake, 2021/02/02
- Re: [PATCH] qemu-img: add seek and -n option to dd command, Peter Lieven, 2021/02/04
- Re: [PATCH] qemu-img: add seek and -n option to dd command, Max Reitz, 2021/02/05
- Re: [PATCH] qemu-img: add seek and -n option to dd command, Peter Lieven, 2021/02/05
- Re: [PATCH] qemu-img: add seek and -n option to dd command, Max Reitz, 2021/02/05
- Re: [PATCH] qemu-img: add seek and -n option to dd command, Max Reitz, 2021/02/05