Re: [Qemu-devel] [PATCH v5 05/10] qemu-io: allow specifying image as a set of options args

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

On Wed, Feb 03, 2016 at 08:37:15AM -0700, Eric Blake wrote:
> On 02/02/2016 05:57 AM, Daniel P. Berrange wrote:
> > Currently qemu-io allows an image filename to be passed on the
> > command line, but unless using the JSON format, it does not have
> > a way to set any options except the format eg
> > 
> >  qemu-io https://127.0.0.1/images/centos7.iso
> >  qemu-io /home/berrange/demo.qcow2
> > 
> > This adds a --image-opts arg that indicates that the positional
> > filename should be interpreted as a full option string, not
> > just a filename.
> > 
> >  qemu-io --image-opts driver=https,url=https://127.0.0.1/images,sslverify=off
> >  qemu-io --image-opts driver=file,filename=/home/berrange/demo.qcow2
> > 
> > This flag is mutually exclusive with the '-f' flag.
> 
> I guess it's easier to enforce the mutual exclusion, than it is to try
> and figure out how to make -f work with the --image-opts filename as
> long as the two aren't specifying conflicting formats.  Seems okay as
> long as it is documented well.
> 
> > 
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  qemu-io.c | 31 ++++++++++++++++++++++++++++++-
> >  1 file changed, 30 insertions(+), 1 deletion(-)
> > 
> 
> > @@ -515,7 +531,20 @@ int main(int argc, char **argv)
> >          flags |= BDRV_O_RDWR;
> >      }
> >  
> > -    if ((argc - optind) == 1) {
> > +    if (imageOpts) {
> > +        QemuOpts *qopts;
> > +        qopts = qemu_opts_parse_noisily(&file_opts, argv[optind], false);
> 
> Ouch. If argc == optind (possible if I type 'qemu-io --image-opts'
> without a filename), then argv[optind] == NULL, and you end up calling
> strncmp(NULL, "id=", 3) inside opts_parse().

Yeah, I should not have removed the ((argc - optind) ==1) check here - it
should be the first thing checked, and imageOpts the second.

> Also, I noticed that running 'qemu-io' without arguments puts you into a
> shell mode, where you can then open files after the fact via the
> open_f() callback function (the 'open' command) - either that function
> should that function be given --image-opts support, or use of
> --image-opts from the command line should globally affect all subsequent
> use of open_f().

That function already has a --option / -o argument that has a similar
result, but I agree that if --image-opts is given on the cli we should
probably use that exclusively.


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