[RFC,v3,for-2.9,07/11] rbd: Clean up qemu_rbd_create()'s detour through QemuOpts

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(-)

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