Re: [PATCH v2 1/1] qemu-img: do not erase destination file in qemu-img dd command

["Daniel P. Berrangé" <berrange@redhat.com>]

On Wed, Nov 01, 2023 at 06:03:36PM +0100, Denis V. Lunev wrote:
> On 11/1/23 17:51, Daniel P. Berrangé wrote:
> > On Tue, Oct 31, 2023 at 03:33:52PM +0100, Hanna Czenczek wrote:
> > > On 01.10.23 22:46, Denis V. Lunev wrote:
> > > > Can you please not top-post. This makes the discussion complex. This
> > > > approach is followed in this mailing list and in other similar lists
> > > > like LKML.
> > > > 
> > > > On 10/1/23 19:08, Mike Maslenkin wrote:
> > > > > I thought about "conv=notrunc", but my main concern is changed virtual
> > > > > disk metadata.
> > > > > It depends on how qemu-img used.
> > > > > May be I followed to wrong pattern, but pros and cons of adding "conv"
> > > > > parameter was not in my mind in scope of the first patch version.
> > > > > I see 4 obvious ways of using `qemu-img dd`:
> > > > > 1. Copy virtual disk data between images of same format. I think disk
> > > > > geometry must be preserved in this case.
> > > > > 2. Copy virtual disk data between different formats. It is a valid
> > > > > pattern? May be `qemu-img convert` should to be used instead?
> > > > > 3. Merge snapshots to specified disk image, i.e read current state and
> > > > > write it to new disk image.
> > > > > 4. Copy virtual disk data to raw binary file. Actually this patch
> > > > > breaks 'dd' behavior for this case when source image is less (in terms
> > > > > of logical blocks) than existed raw binary file.
> > > > >       May be for this case condition can be improved to smth like
> > > > >      if (strcmp(fmt, "raw") || !g_file_test(out.filename,
> > > > > G_FILE_TEST_EXISTS)) . And parameter "conv=notrunc" may be implemented
> > > > > additionally for this case.
> > > > My personal opinion is that qemu dd when you will need to
> > > > extract the SOME data from the original image and process
> > > > it further. Thus I use it to copy some data into raw binary
> > > > file. My next goal here would add ability to put data into
> > > > stdout that would be beneficial for. Though this is out of the
> > > > equation at the moment.
> > > > 
> > > > Though, speaking about the approach, I would say that the
> > > > patch changes current behavior which is not totally buggy
> > > > under a matter of this or that taste. It should be noted that
> > > > we are here in Linux world, not in the Mac world where we
> > > > were in position to avoid options and selections.
> > > > 
> > > > Thus my opinion that original behavior is to be preserved
> > > > as somebody is relying on it. The option you are proposing
> > > > seems valuable to me also and thus the switch is to be added.
> > > > The switch is well-defined in the original 'dd' world thus
> > > > either conv= option would be good, either nocreat or notrunc.
> > > > For me 'nocreat' seems more natural.
> > > > 
> > > > Anyway, the last word here belongs to either Hanna or Kevin ;)
> > > Personally, and honestly, I see no actual use for qemu-img dd at all,
> > > because we’re trying to mimic a subset of an interface of a rather complex
> > > program that has been designed to do what it does. We can only fail at
> > > that.  Personally, whenever I need dd functionality, I use
> > > qemu-storage-daemon’s fuse export, and then use the actual dd program on
> > > top.  Alternatively, qemu-img convert is our native interface;
> > > unfortunately, its feature set is lacking when compared to qemu-img dd, but
> > > I think it would be better to improve that rather than working on qemu-img
> > > dd.
> > Is there a clear view of what gaps exist in 'qemu-img convert', and more
> > importantly, how much work is it to close the gaps, such that 'dd' could
> > potentially be deprecated & eventually removed ?
> > 
> I am using 'qemu-img dd' as a way to get (some) content
> from the image. I have dreamed about getting it to
> stdout.

FWIW, you can use qemu-img convert to extract a subset of data from an
image by layering in the 'raw' format.

  qemu-img convert --image-opts \
      driver=raw,offset=1024,size=512,file.driver=file,file.filename=demo.img data.bin

somewhat annoyingly it forces 'size' to be a multiple of 512. That makes
sense if you're using the output as backing for a VM disk, but for simply
extracting data, conceptually it shouldn't be needed.

Yes, the syntax gets hairy with image opts :-)

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|