[Qemu-devel] [PATCH v3 2/4] qemu-img: fix --image-opts usage with dd command

Daniel P. Berrange posted 4 patches 8 years, 11 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v3 2/4] qemu-img: fix --image-opts usage with dd command
Posted by Daniel P. Berrange 8 years, 11 months ago
The --image-opts flag can only be used to affect the parsing
of the source image. The target image has to be specified in
the traditional style regardless, since it needs to be passed
to the bdrv_create() API which does not support the new style
opts.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 qemu-img.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 739345e..d8a737f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4102,8 +4102,13 @@ static int img_dd(int argc, char **argv)
         goto out;
     }
 
-    blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
-                    false, false);
+    /* TODO, we can't honour --image-opts for the target,
+     * since it needs to be given in a format compatible
+     * with the bdrv_create() call above which does not
+     * support image-opts style.
+     */
+    blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR,
+                         false, false);
 
     if (!blk2) {
         ret = -1;
-- 
2.9.3


Re: [Qemu-devel] [PATCH v3 2/4] qemu-img: fix --image-opts usage with dd command
Posted by Kevin Wolf 8 years, 11 months ago
Am 20.02.2017 um 16:19 hat Daniel P. Berrange geschrieben:
> The --image-opts flag can only be used to affect the parsing
> of the source image. The target image has to be specified in
> the traditional style regardless, since it needs to be passed
> to the bdrv_create() API which does not support the new style
> opts.
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

Hm. This means that...

1. --image-opts never worked for 'qemu-img dd'

2. If we ever change bdrv_create() to be more flexible, with this patch
   we'd be stuck with an inconsistent "filename for target, options for
   source" interface because we can't change the semantics any more.

Should we just remove --image-opts from qemu-img dd instead until we can
provide the real thing?

Kevin

Re: [Qemu-devel] [PATCH v3 2/4] qemu-img: fix --image-opts usage with dd command
Posted by Daniel P. Berrange 8 years, 11 months ago
On Wed, Feb 22, 2017 at 11:46:06AM +0100, Kevin Wolf wrote:
> Am 20.02.2017 um 16:19 hat Daniel P. Berrange geschrieben:
> > The --image-opts flag can only be used to affect the parsing
> > of the source image. The target image has to be specified in
> > the traditional style regardless, since it needs to be passed
> > to the bdrv_create() API which does not support the new style
> > opts.
> > 
> > Reviewed-by: Max Reitz <mreitz@redhat.com>
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> 
> Hm. This means that...
> 
> 1. --image-opts never worked for 'qemu-img dd'
> 
> 2. If we ever change bdrv_create() to be more flexible, with this patch
>    we'd be stuck with an inconsistent "filename for target, options for
>    source" interface because we can't change the semantics any more.
> 
> Should we just remove --image-opts from qemu-img dd instead until we can
> provide the real thing?

We're already in that situation wrt bdrv_create() for other commands, so
we need a separate flag to request use of image opts for the target
image. So I don't think we want to special case dd in this respect.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|