Re: [Qemu-block] [PATCH for-2.7? 1/5] block/ssh: Use QemuOpts for runtime options

[Kevin Wolf]

Am 12.08.2016 um 19:15 hat Max Reitz geschrieben:
> Using QemuOpts will prevent qemu from crashing if the input options have
> not been validated (which is the case when they are specified on the
> command line or in a json: filename) and some have the wrong type.
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block/ssh.c | 77 
> ++++++++++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 53 insertions(+), 24 deletions(-)
> 
> diff --git a/block/ssh.c b/block/ssh.c
> index bcbb0e4..4b90f34 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -508,36 +508,73 @@ static int authenticate(BDRVSSHState *s, const char 
> *user, Error **errp)
>      return ret;
>  }
>  
> +static QemuOptsList runtime_opts = {

I would generally prefer some prefix to make the name unique instead of
having something called "runtime_opts" in every driver. Currently we're
inconsistent, but most driver do use a prefix.

> @@ -618,15 +654,6 @@ static int connect_to_ssh(BDRVSSHState *s, QDict 
> *options,
>          return -EINVAL;
>      }
>  
> -    /* Delete the options we've used; any not deleted will cause the
> -     * block layer to give an error about unused options.
> -     */
> -    qdict_del(options, "host");
> -    qdict_del(options, "port");
> -    qdict_del(options, "user");
> -    qdict_del(options, "path");
> -    qdict_del(options, "host_key_check");
> -
>      return 0;
>  
>   err:
> @@ -646,6 +673,8 @@ static int connect_to_ssh(BDRVSSHState *s, QDict *options,
>      }
>      s->session = NULL;
>  
> +    qemu_opts_del(opts);
> +
>      return ret;
>  }

opts is leaked in the normal path and only freed on error.

Kevin