Message ID | 1490621195-2228-8-git-send-email-armbru@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
On 27.03.2017 15:26, Markus Armbruster wrote: > The conversion from QDict to QemuOpts is pointless. Simply get the > stuff straight from the QDict. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Kevin Wolf <kwolf@redhat.com> > --- > block/rbd.c | 20 +++++--------------- > 1 file changed, 5 insertions(+), 15 deletions(-) > > diff --git a/block/rbd.c b/block/rbd.c > index b2afe07..cf0bab0 100644 > --- a/block/rbd.c > +++ b/block/rbd.c > @@ -376,7 +376,6 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp) > rados_t cluster; > rados_ioctx_t io_ctx; > QDict *options = NULL; > - QemuOpts *rbd_opts = NULL; > int ret = 0; > > secretid = qemu_opt_get(opts, "password-secret"); > @@ -407,19 +406,11 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp) > goto exit; > } > > - rbd_opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); > - qemu_opts_absorb_qdict(rbd_opts, options, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > - ret = -EINVAL; > - goto exit; > - } > - > - pool = qemu_opt_get(rbd_opts, "pool"); > - conf = qemu_opt_get(rbd_opts, "conf"); > - clientname = qemu_opt_get(rbd_opts, "user"); > - name = qemu_opt_get(rbd_opts, "image"); > - keypairs = qemu_opt_get(rbd_opts, "=keyvalue-pairs"); > + pool = qdict_get_str(options, "pool"); > + conf = qdict_get_str(options, "conf"); > + clientname = qdict_get_str(options, "user"); > + name = qdict_get_str(options, "image"); > + keypairs = qdict_get_str(options, "=keyvalue-pairs"); This assumes that all of these options are given which they are not necessarily: $ ./qemu-img create -f rbd rbd:blub/ 1M Formatting 'rbd:blub/', fmt=rbd size=1048576 [1] 27387 segmentation fault (core dumped) ./qemu-img create -f rbd rbd:blub/ 1M qdict_get_try_str() would probably be better. Max > > ret = rados_create(&cluster, clientname); > if (ret < 0) { > @@ -470,7 +461,6 @@ shutdown: > > exit: > QDECREF(options); > - qemu_opts_del(rbd_opts); > return ret; > } > >
Max Reitz <mreitz@redhat.com> writes: > On 27.03.2017 15:26, Markus Armbruster wrote: >> The conversion from QDict to QemuOpts is pointless. Simply get the >> stuff straight from the QDict. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> Reviewed-by: Kevin Wolf <kwolf@redhat.com> >> --- >> block/rbd.c | 20 +++++--------------- >> 1 file changed, 5 insertions(+), 15 deletions(-) >> >> diff --git a/block/rbd.c b/block/rbd.c >> index b2afe07..cf0bab0 100644 >> --- a/block/rbd.c >> +++ b/block/rbd.c >> @@ -376,7 +376,6 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp) >> rados_t cluster; >> rados_ioctx_t io_ctx; >> QDict *options = NULL; >> - QemuOpts *rbd_opts = NULL; >> int ret = 0; >> >> secretid = qemu_opt_get(opts, "password-secret"); >> @@ -407,19 +406,11 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp) >> goto exit; >> } >> >> - rbd_opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); >> - qemu_opts_absorb_qdict(rbd_opts, options, &local_err); >> - if (local_err) { >> - error_propagate(errp, local_err); >> - ret = -EINVAL; >> - goto exit; >> - } >> - >> - pool = qemu_opt_get(rbd_opts, "pool"); >> - conf = qemu_opt_get(rbd_opts, "conf"); >> - clientname = qemu_opt_get(rbd_opts, "user"); >> - name = qemu_opt_get(rbd_opts, "image"); >> - keypairs = qemu_opt_get(rbd_opts, "=keyvalue-pairs"); >> + pool = qdict_get_str(options, "pool"); >> + conf = qdict_get_str(options, "conf"); >> + clientname = qdict_get_str(options, "user"); >> + name = qdict_get_str(options, "image"); >> + keypairs = qdict_get_str(options, "=keyvalue-pairs"); > > This assumes that all of these options are given which they are not > necessarily: > > $ ./qemu-img create -f rbd rbd:blub/ 1M > Formatting 'rbd:blub/', fmt=rbd size=1048576 > [1] 27387 segmentation fault (core dumped) ./qemu-img create -f rbd > rbd:blub/ 1M > > qdict_get_try_str() would probably be better. Yes, will fix. I've fallen into this trap before...
diff --git a/block/rbd.c b/block/rbd.c index b2afe07..cf0bab0 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -376,7 +376,6 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp) rados_t cluster; rados_ioctx_t io_ctx; QDict *options = NULL; - QemuOpts *rbd_opts = NULL; int ret = 0; secretid = qemu_opt_get(opts, "password-secret"); @@ -407,19 +406,11 @@ static int qemu_rbd_create(const char *filename, QemuOpts *opts, Error **errp) goto exit; } - rbd_opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort); - qemu_opts_absorb_qdict(rbd_opts, options, &local_err); - if (local_err) { - error_propagate(errp, local_err); - ret = -EINVAL; - goto exit; - } - - pool = qemu_opt_get(rbd_opts, "pool"); - conf = qemu_opt_get(rbd_opts, "conf"); - clientname = qemu_opt_get(rbd_opts, "user"); - name = qemu_opt_get(rbd_opts, "image"); - keypairs = qemu_opt_get(rbd_opts, "=keyvalue-pairs"); + pool = qdict_get_str(options, "pool"); + conf = qdict_get_str(options, "conf"); + clientname = qdict_get_str(options, "user"); + name = qdict_get_str(options, "image"); + keypairs = qdict_get_str(options, "=keyvalue-pairs"); ret = rados_create(&cluster, clientname); if (ret < 0) { @@ -470,7 +461,6 @@ shutdown: exit: QDECREF(options); - qemu_opts_del(rbd_opts); return ret; }