Re: [PATCH 0/4] Make qemu-img dd more flexible

[Fabian Ebner]

Am 11.02.22 um 17:42 schrieb Hanna Reitz:
> On 11.02.22 17:31, Eric Blake wrote:
>> On Thu, Feb 10, 2022 at 02:31:19PM +0100, Fabian Ebner wrote:
>>> Adds support for reading from stdin and writing to stdout (when raw
>>> format is used), as well as overriding the size of the output and
>>> input image/stream.
>>>
>>> Additionally, the options -n for skipping output image creation and -l
>>> for loading a snapshot are made available like for convert.
>> Without looking at the series itself, I want to refer back to earlier
>> times that someone proposed improving 'qemu-img dd':
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2021-02/msg00636.html
>> https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg02618.html
>>
>> As well as the observation that when we originally allowed 'qemu-img
>> dd' to be added, the end goal was that if 'qemu-img dd' can't operate
>> as a thin wrapper around 'qemu-img convert', then 'qemu-img convert'
>> needs to be made more powerful first.  Every time we diverge on what
>> the two uses can do, rather than keeping dd as a thin wrapper, we add
>> to our maintenance burden.

I'm wondering why it's not actually implemented as a thin wrapper then?
The fact that it isn't is (part of) the reason why dd was chosen, as
mentioned in the first patch:

"While dd and convert have overlapping use cases, `dd` is a
simple read/write loop while convert is much more
sophisticated and has ways to dealing with holes and blocks
of zeroes.
Since these typically can't be detected in pipes via
SEEK_DATA/HOLE or skipped while writing, dd seems to be the
better choice for implementing stdin/stdout streams."

Adding the same feature to convert seems much more involved.

>>
>> Sadly, there is a lot of technical debt in this area ('qemu-img dd
>> skip= count=' is STILL broken, more than 4 years after I first
>> proposed a potential patch), where no one has spent the necessary time
>> to improve the situation.
> 
> Note that by now (in contrast to 2018), we have FUSE disk exports, and I
> even have a script that uses them to let you run dd on any image:
> 
> https://gitlab.com/hreitz/qemu-scripts/-/blob/main/qemu-dd.py
> 
> Which is nice, because it gives you feature parity with dd, because it
> simply runs dd.

Thank you for the suggestion. It's definitely worth considering,
although it does add a bit of complexity and we currently don't have
FUSE support enabled in our builds.

> 
> (The main problem with the script is that it lives in that personal repo
> of mine and so nobody but me knows about it.  Suggestions to improve
> that situation are more than welcome.)
> 
> Now, the qemu storage daemon does not support loading qcow2 snapshots
> (as far as I’m aware), which is proposed in patch 4 of this series.  But
> I think that just means that it would be nice if the QSD could support
> that.

I suppose adding a snapshot-load QMP command would be a natural way to
add it?

> 
> Hanna
> 
>