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

Commit Message

Max Reitz Aug. 12, 2016, 5:15 p.m. UTC

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 <[e-mail]>
---
 block/ssh.c | 77 ++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 53 insertions(+), 24 deletions(-)

Comments

Kevin Wolf Aug. 15, 2016, 9:20 a.m. UTC | #1

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 <[e-mail]>
> ---
>  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

Patch

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 = {
+    .name = "ssh",
+    .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
+    .desc = {
+        {
+            .name = "host",
+            .type = QEMU_OPT_STRING,
+            .help = "Host to connect to",
+        },
+        {
+            .name = "port",
+            .type = QEMU_OPT_NUMBER,
+            .help = "Port to connect to",
+        },
+        {
+            .name = "path",
+            .type = QEMU_OPT_STRING,
+            .help = "Path of the image on the host",
+        },
+        {
+            .name = "user",
+            .type = QEMU_OPT_STRING,
+            .help = "User as which to connect",
+        },
+        {
+            .name = "host_key_check",
+            .type = QEMU_OPT_STRING,
+            .help = "Defines how and what to check the host key against",
+        },
+    },
+};
+
 static int connect_to_ssh(BDRVSSHState *s, QDict *options,
                           int ssh_flags, int creat_mode, Error **errp)
 {
     int r, ret;
+    QemuOpts *opts = NULL;
+    Error *local_err = NULL;
     const char *host, *user, *path, *host_key_check;
     int port;
 
-    if (!qdict_haskey(options, "host")) {
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
+    qemu_opts_absorb_qdict(opts, options, &local_err);
+    if (local_err) {
         ret = -EINVAL;
-        error_setg(errp, "No hostname was specified");
+        error_propagate(errp, local_err);
         goto err;
     }
-    host = qdict_get_str(options, "host");
 
-    if (qdict_haskey(options, "port")) {
-        port = qdict_get_int(options, "port");
-    } else {
-        port = 22;
+    host = qemu_opt_get(opts, "host");
+    if (!host) {
+        ret = -EINVAL;
+        error_setg(errp, "No hostname was specified");
+        goto err;
     }
 
-    if (!qdict_haskey(options, "path")) {
+    port = qemu_opt_get_number(opts, "port", 22);
+
+    path = qemu_opt_get(opts, "path");
+    if (!path) {
         ret = -EINVAL;
         error_setg(errp, "No path was specified");
         goto err;
     }
-    path = qdict_get_str(options, "path");
 
-    if (qdict_haskey(options, "user")) {
-        user = qdict_get_str(options, "user");
-    } else {
+    user = qemu_opt_get(opts, "user");
+    if (!user) {
         user = g_get_user_name();
         if (!user) {
             error_setg_errno(errp, errno, "Can't get user name");
@@ -546,9 +583,8 @@  static int connect_to_ssh(BDRVSSHState *s, QDict *options,
         }
     }
 
-    if (qdict_haskey(options, "host_key_check")) {
-        host_key_check = qdict_get_str(options, "host_key_check");
-    } else {
+    host_key_check = qemu_opt_get(opts, "host_key_check");
+    if (!host_key_check) {
         host_key_check = "yes";
     }
 
@@ -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;
 }