diff mbox

[RFC,2/6] iscsi: Handle -iscsi user/password in bdrv_parse_filename()

Message ID 1481203391-9523-3-git-send-email-kwolf@redhat.com
State New
Headers show

Commit Message

Kevin Wolf Dec. 8, 2016, 1:23 p.m. UTC 
This splits the logic in the old parse_chap() function into a part that
parses the -iscsi options into the new driver-specific options, and
another part that actually applies those options (called apply_chap()
now).

Note that this means that username and password specified with -iscsi
only take effect when a URL is provided. This is intentional, -iscsi is
a legacy interface only supported for compatibility, new users should
use the proper driver-specific options.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/iscsi.c | 72 +++++++++++++++++++++++++++++++----------------------------
 1 file changed, 38 insertions(+), 34 deletions(-)

Comments

Daniel P. Berrangé Dec. 8, 2016, 1:42 p.m. UTC | #1 
On Thu, Dec 08, 2016 at 02:23:07PM +0100, Kevin Wolf wrote:
> This splits the logic in the old parse_chap() function into a part that
> parses the -iscsi options into the new driver-specific options, and
> another part that actually applies those options (called apply_chap()
> now).
> 
> Note that this means that username and password specified with -iscsi
> only take effect when a URL is provided. This is intentional, -iscsi is
> a legacy interface only supported for compatibility, new users should
> use the proper driver-specific options.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/iscsi.c | 72 +++++++++++++++++++++++++++++++----------------------------
>  1 file changed, 38 insertions(+), 34 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 7a6664e..c5106c1 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1235,29 +1235,14 @@ retry:
>      return 0;
>  }
>  
> -static void parse_chap(struct iscsi_context *iscsi, const char *target,
> +static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
>                         Error **errp)
>  {
> -    QemuOptsList *list;
> -    QemuOpts *opts;
>      const char *user = NULL;
>      const char *password = NULL;
>      const char *secretid;
>      char *secret = NULL;
>  
> -    list = qemu_find_opts("iscsi");
> -    if (!list) {
> -        return;
> -    }
> -
> -    opts = qemu_opts_find(list, target);
> -    if (opts == NULL) {
> -        opts = QTAILQ_FIRST(&list->head);
> -        if (!opts) {
> -            return;
> -        }
> -    }
> -
>      user = qemu_opt_get(opts, "user");
>      if (!user) {
>          return;
> @@ -1586,6 +1571,35 @@ out:
>      }
>  }
>  
> +static void iscsi_parse_iscsi_option(const char *target, QDict *options)
> +{
> +    QemuOptsList *list;
> +    QemuOpts *opts;
> +    const char *user;
> +
> +    list = qemu_find_opts("iscsi");
> +    if (!list) {
> +        return;
> +    }
> +
> +    opts = qemu_opts_find(list, target);
> +    if (opts == NULL) {
> +        opts = QTAILQ_FIRST(&list->head);
> +        if (!opts) {
> +            return;
> +        }
> +    }
> +
> +    user = qemu_opt_get(opts, "user");
> +    if (user) {
> +        qdict_set_default_str(options, "user", user);
> +        qdict_set_default_str(options, "password",
> +                              qemu_opt_get(opts, "password"));
> +        qdict_set_default_str(options, "password-secret",
> +                              qemu_opt_get(opts, "password-secret"));

This core dumps if you set '-iscsi user=foo' but don't set password
or password-secret

#0  0x00007fffda7e2046 in strlen () at /lib64/libc.so.6
#1  0x0000555555b012f9 in qstring_from_str (str=str@entry=0x0) at qobject/qstring.c:66
#2  0x0000555555b02148 in qdict_set_default_str (dst=dst@entry=0x5555566ea7d0, key=key@entry=0x555555b2238b "password", val=0x0) at qobject/qdict.c:490
#3  0x0000555555ab515f in iscsi_parse_iscsi_option (options=0x5555566ea7d0, target=0x5555566f0eb0 "iqn.2004-04.fedora:fedora25:iscsi.kvm") at block/iscsi.c:1545
#4  0x0000555555ab515f in iscsi_parse_filename (filename=<optimized out>, options=0x5555566ea7d0, errp=<optimized out>) at block/iscsi.c:1610


Regards,
Daniel
diff mbox

Patch

diff --git a/block/iscsi.c b/block/iscsi.c
index 7a6664e..c5106c1 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1235,29 +1235,14 @@  retry:
     return 0;
 }
 
-static void parse_chap(struct iscsi_context *iscsi, const char *target,
+static void apply_chap(struct iscsi_context *iscsi, QemuOpts *opts,
                        Error **errp)
 {
-    QemuOptsList *list;
-    QemuOpts *opts;
     const char *user = NULL;
     const char *password = NULL;
     const char *secretid;
     char *secret = NULL;
 
-    list = qemu_find_opts("iscsi");
-    if (!list) {
-        return;
-    }
-
-    opts = qemu_opts_find(list, target);
-    if (opts == NULL) {
-        opts = QTAILQ_FIRST(&list->head);
-        if (!opts) {
-            return;
-        }
-    }
-
     user = qemu_opt_get(opts, "user");
     if (!user) {
         return;
@@ -1586,6 +1571,35 @@  out:
     }
 }
 
+static void iscsi_parse_iscsi_option(const char *target, QDict *options)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const char *user;
+
+    list = qemu_find_opts("iscsi");
+    if (!list) {
+        return;
+    }
+
+    opts = qemu_opts_find(list, target);
+    if (opts == NULL) {
+        opts = QTAILQ_FIRST(&list->head);
+        if (!opts) {
+            return;
+        }
+    }
+
+    user = qemu_opt_get(opts, "user");
+    if (user) {
+        qdict_set_default_str(options, "user", user);
+        qdict_set_default_str(options, "password",
+                              qemu_opt_get(opts, "password"));
+        qdict_set_default_str(options, "password-secret",
+                              qemu_opt_get(opts, "password-secret"));
+    }
+}
+
 /*
  * We support iscsi url's on the form
  * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
@@ -1624,6 +1638,9 @@  static void iscsi_parse_filename(const char *filename, QDict *options,
     qdict_set_default_str(options, "lun", lun_str);
     g_free(lun_str);
 
+    /* User/password from -iscsi take precedence over those from the URL */
+    iscsi_parse_iscsi_option(iscsi_url->target, options);
+
     if (iscsi_url->user[0] != '\0') {
         qdict_set_default_str(options, "user", iscsi_url->user);
         qdict_set_default_str(options, "password", iscsi_url->passwd);
@@ -1658,6 +1675,10 @@  static QemuOptsList runtime_opts = {
             .type = QEMU_OPT_STRING,
         },
         {
+            .name = "password-secret",
+            .type = QEMU_OPT_STRING,
+        },
+        {
             .name = "lun",
             .type = QEMU_OPT_NUMBER,
         },
@@ -1677,7 +1698,6 @@  static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
     QemuOpts *opts;
     Error *local_err = NULL;
     const char *transport_name, *portal, *target;
-    const char *user, *password;
 #if LIBISCSI_API_VERSION >= (20160603)
     enum iscsi_transport_type transport;
 #endif
@@ -1694,8 +1714,6 @@  static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
     transport_name = qemu_opt_get(opts, "transport");
     portal = qemu_opt_get(opts, "portal");
     target = qemu_opt_get(opts, "target");
-    user = qemu_opt_get(opts, "user");
-    password = qemu_opt_get(opts, "password");
     lun = qemu_opt_get_number(opts, "lun", 0);
 
     if (!transport_name || !portal || !target) {
@@ -1703,11 +1721,6 @@  static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
         ret = -EINVAL;
         goto out;
     }
-    if (user && !password) {
-        error_setg(errp, "If a user name is given, a password is required");
-        ret = -EINVAL;
-        goto out;
-    }
 
     if (!strcmp(transport_name, "tcp")) {
 #if LIBISCSI_API_VERSION >= (20160603)
@@ -1746,17 +1759,8 @@  static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
         goto out;
     }
 
-    if (user) {
-        ret = iscsi_set_initiator_username_pwd(iscsi, user, password);
-        if (ret != 0) {
-            error_setg(errp, "Failed to set initiator username and password");
-            ret = -EINVAL;
-            goto out;
-        }
-    }
-
     /* check if we got CHAP username/password via the options */
-    parse_chap(iscsi, target, &local_err);
+    apply_chap(iscsi, opts, &local_err);
     if (local_err != NULL) {
         error_propagate(errp, local_err);
         ret = -EINVAL;