[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