Re: [PATCH] qemu-img: add seek and -n option to dd command

[Eric Blake <eblake@redhat.com>]

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