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

[Daniel P. Berrange]

On Wed, Jan 27, 2016 at 03:26:51PM +0100, Kevin Wolf wrote:
> Am 26.01.2016 um 14:34 hat Daniel P. Berrange geschrieben:
> > -    if ((argc - optind) == 1) {
> > +    if (imageOpts) {
> > +        char *file;
> > +        qopts = qemu_opts_parse_noisily(&file_opts, argv[optind], false);
> > +        if (!qopts) {
> > +            exit(1);
> > +        }
> > +        if (opts) {
> > +            error_report("--image-opts and -f are mutually exclusive");
> > +            exit(1);
> > +        }
> > +        file = g_strdup(qemu_opt_get(qopts, "file"));
> > +        qemu_opt_unset(qopts, "file");
> 
> I'm not sure if special casing "file" is a good idea. I think you're
> doing it in order to support something that looks like -drive, but it's
> not the same anyway - all of the options that are parsed in drive_new()
> or blockdev_init() aren't supported.
> 
> We could change things so that qemu-io actually goes through
> drive_new() or blockdev_init(), at the cost of forbidding node
> references for the "file" options like -drive unfortunately does. Or we
> can require that you use actual bdrv_open() options and specify
> file.filename=... for raw-posix.
> 
> But I would like to avoid creating a new type of option parsing with yet
> another behaviour.

Mostly this special casing of 'file' was based on my confusion /
misunderstanding of the recommended best API usage around bdrv_open.
I thought I was being compatible with -drive from the system emulators
by doing this, but you point out I'm not actually doing it properly.

Lets just keep this simple and use bdrv_open() options explicitly
since IIUC that looks like the "best practice" approach these days


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 :|