| Message ID | 1453815262-13440-6-git-send-email-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 26.01.2016 um 14:34 hat Daniel P. Berrange geschrieben: > 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=http,url=https://127.0.0.1/images,sslverify=off > qemu-io --image-opts file=/home/berrange/demo.qcow2 > > This flag is mutually exclusive with the '-f' flag. > > Signed-off-by: Daniel P. Berrange <berrange@redhat.com> > --- > qemu-io.c | 34 +++++++++++++++++++++++++++++++++- > 1 file changed, 33 insertions(+), 1 deletion(-) > > diff --git a/qemu-io.c b/qemu-io.c > index d1432ea..51d8272 100644 > --- a/qemu-io.c > +++ b/qemu-io.c > @@ -367,6 +367,7 @@ static void reenable_tty_echo(void) > > enum { > OPTION_OBJECT = 256, > + OPTION_IMAGE_OPTS = 257, > }; > > static QemuOptsList qemu_object_opts = { > @@ -397,6 +398,16 @@ static int object_create(void *opaque, QemuOpts *opts, Error **errp) > return 0; > } > > +static QemuOptsList file_opts = { > + .name = "file", > + .implied_opt_name = "file", > + .head = QTAILQ_HEAD_INITIALIZER(file_opts.head), > + .desc = { > + /* no elements => accept any params */ > + { /* end of list */ } > + }, > +}; > + > int main(int argc, char **argv) > { > int readonly = 0; > @@ -416,6 +427,7 @@ int main(int argc, char **argv) > { "cache", 1, NULL, 't' }, > { "trace", 1, NULL, 'T' }, > { "object", 1, NULL, OPTION_OBJECT }, > + { "image-opts", 0, NULL, OPTION_IMAGE_OPTS }, > { NULL, 0, NULL, 0 } > }; > int c; > @@ -424,6 +436,7 @@ int main(int argc, char **argv) > Error *local_error = NULL; > QDict *opts = NULL; > QemuOpts *qopts = NULL; > + bool imageOpts = false; > > #ifdef CONFIG_POSIX > signal(SIGPIPE, SIG_IGN); > @@ -492,6 +505,9 @@ int main(int argc, char **argv) > exit(1); > } > break; > + case OPTION_IMAGE_OPTS: > + imageOpts = true; > + break; > default: > usage(progname); > exit(1); > @@ -534,7 +550,23 @@ int main(int argc, char **argv) > flags |= BDRV_O_RDWR; > } > > - 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. > + opts = qemu_opts_to_qdict(qopts, NULL); > + qemu_opts_reset(&file_opts); > + openfile(file, flags, opts); > + g_free(file); > + } else if ((argc - optind) == 1) { > openfile(argv[optind], flags, opts); > } > command_loop(); Kevin
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
diff --git a/qemu-io.c b/qemu-io.c index d1432ea..51d8272 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -367,6 +367,7 @@ static void reenable_tty_echo(void) enum { OPTION_OBJECT = 256, + OPTION_IMAGE_OPTS = 257, }; static QemuOptsList qemu_object_opts = { @@ -397,6 +398,16 @@ static int object_create(void *opaque, QemuOpts *opts, Error **errp) return 0; } +static QemuOptsList file_opts = { + .name = "file", + .implied_opt_name = "file", + .head = QTAILQ_HEAD_INITIALIZER(file_opts.head), + .desc = { + /* no elements => accept any params */ + { /* end of list */ } + }, +}; + int main(int argc, char **argv) { int readonly = 0; @@ -416,6 +427,7 @@ int main(int argc, char **argv) { "cache", 1, NULL, 't' }, { "trace", 1, NULL, 'T' }, { "object", 1, NULL, OPTION_OBJECT }, + { "image-opts", 0, NULL, OPTION_IMAGE_OPTS }, { NULL, 0, NULL, 0 } }; int c; @@ -424,6 +436,7 @@ int main(int argc, char **argv) Error *local_error = NULL; QDict *opts = NULL; QemuOpts *qopts = NULL; + bool imageOpts = false; #ifdef CONFIG_POSIX signal(SIGPIPE, SIG_IGN); @@ -492,6 +505,9 @@ int main(int argc, char **argv) exit(1); } break; + case OPTION_IMAGE_OPTS: + imageOpts = true; + break; default: usage(progname); exit(1); @@ -534,7 +550,23 @@ int main(int argc, char **argv) flags |= BDRV_O_RDWR; } - 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"); + opts = qemu_opts_to_qdict(qopts, NULL); + qemu_opts_reset(&file_opts); + openfile(file, flags, opts); + g_free(file); + } else if ((argc - optind) == 1) { openfile(argv[optind], flags, opts); } command_loop();
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=http,url=https://127.0.0.1/images,sslverify=off qemu-io --image-opts file=/home/berrange/demo.qcow2 This flag is mutually exclusive with the '-f' flag. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- qemu-io.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-)