qemu-devel
[Top][All Lists]
Advanced

[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: Peter Lieven
Subject: Re: [PATCH] qemu-img: add seek and -n option to dd command
Date: Thu, 4 Feb 2021 21:09:53 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0

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.


>
> 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.


>
> 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,

Peter






reply via email to

[Prev in Thread] Current Thread [Next in Thread]