qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/0] Add configuration variables for iSCSI
@ 2012-01-25 22:39 Ronnie Sahlberg
  2012-01-25 22:39 ` [Qemu-devel] [PATCH] iSCSI: add " Ronnie Sahlberg
  0 siblings, 1 reply; 4+ messages in thread
From: Ronnie Sahlberg @ 2012-01-25 22:39 UTC (permalink / raw)
  To: qemu-devel, kwolf, eblake

Kevin, List
Please review and/or apply.


This is version 3 of the patch to add configuration variables for iSCSI.
Version 2 added the feature to specify configuration blocks that apply to a specific target name, allowing qemu to use different settings if/when connecting one guest to multiple different iscsi targets.
Version 3 fixes a typo and adds an example to the documentation on how to use -readconfig for reading the configuration from a file


This patch adds configuration variables for iSCSI to set
initiator-name to use when logging in to the target,
which type of header-digest to negotiate with the target
and username and password for CHAP authentication.

This allows specifying a initiator-name either from the command line
-iscsi initiator-name=iqn.2004-01.com.example:test
or from a configuration file included with -readconfig
[iscsi]
 initiator-name = iqn.2004-01.com.example:test
 header-digest = CRC32C|CRC32C-NONE|NONE-CRC32C|NONE
 user = CHAP username
 password = CHAP password

In the configuration file it is also possible to set a target specific
configuratyion block using the header
[iscsi "iqn.target.name"]

When a iscsi session is initialized, it will first try to use a configuration
section that matches the target name.
If no such block is found, it will fall-back to try the default [iscsi] section instead.

regards
ronnie sahlberg

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI
  2012-01-25 22:39 [Qemu-devel] [PATCH 0/0] Add configuration variables for iSCSI Ronnie Sahlberg
@ 2012-01-25 22:39 ` Ronnie Sahlberg
  2012-01-25 22:53   ` Eric Blake
  2012-01-26 10:58   ` Kevin Wolf
  0 siblings, 2 replies; 4+ messages in thread
From: Ronnie Sahlberg @ 2012-01-25 22:39 UTC (permalink / raw)
  To: qemu-devel, kwolf, eblake; +Cc: Ronnie Sahlberg

This patch adds configuration variables for iSCSI to set
initiator-name to use when logging in to the target,
which type of header-digest to negotiate with the target
and username and password for CHAP authentication.

This allows specifying a initiator-name either from the command line
-iscsi initiator-name=iqn.2004-01.com.example:test
or from a configuration file included with -readconfig
    [iscsi]
      initiator-name = iqn.2004-01.com.example:test
      header-digest = CRC32C|CRC32C-NONE|NONE-CRC32C|NONE
      user = CHAP username
      password = CHAP password

If you use several different targets, you can also configure this on a per
target basis by using a group name:
    [iscsi "iqn.target.name"]
    ...

The configuration file can be read using -readconfig.
Example :
qemu-system-i386 -drive file=iscsi://127.0.0.1/iqn.ronnie.test/1
 -readconfig iscsi.conf

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 block/iscsi.c   |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 qemu-config.c   |   27 +++++++++++
 qemu-doc.texi   |   54 +++++++++++++++++++++-
 qemu-options.hx |   16 +++++--
 vl.c            |    8 +++
 5 files changed, 229 insertions(+), 15 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 938c568..bd3ca11 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -455,6 +455,109 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
     }
 }
 
+static int parse_chap(struct iscsi_context *iscsi, const char *target)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const char *user = NULL;
+    const char *password = NULL;
+
+    list = qemu_find_opts("iscsi");
+    if (!list) {
+        return 0;
+    }
+
+    opts = qemu_opts_find(list, target);
+    if (opts == NULL) {
+        opts = QTAILQ_FIRST(&list->head);
+        if (!opts) {
+            return 0;
+        }
+    }
+
+    user = qemu_opt_get(opts, "user");
+    if (!user) {
+        return 0;
+    }
+
+    password = qemu_opt_get(opts, "password");
+    if (!password) {
+        error_report("CHAP username specified but no password was given");
+        return -1;
+    }
+
+    if (iscsi_set_initiator_username_pwd(iscsi, user, password)) {
+        error_report("Failed to set initiator username and password");
+        return -1;
+    }
+
+    return 0;
+}
+
+static void parse_header_digest(struct iscsi_context *iscsi, const char *target)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const char *digest = 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;
+        }
+    }
+
+    digest = qemu_opt_get(opts, "header-digest");
+    if (!digest) {
+        return;
+    }
+
+    if (!strcmp(digest, "CRC32C")) {
+        iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
+    } else if (!strcmp(digest, "NONE")) {
+        iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE);
+    } else if (!strcmp(digest, "CRC32C-NONE")) {
+        iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C_NONE);
+    } else if (!strcmp(digest, "NONE-CRC32C")) {
+        iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
+    } else {
+        error_report("Invalid header-digest setting : %s", digest);
+    }
+}
+
+static char *parse_initiator_name(const char *target)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const char *name = NULL;
+
+    list = qemu_find_opts("iscsi");
+    if (!list) {
+        return g_strdup("iqn.2008-11.org.linux-kvm");
+    }
+
+    opts = qemu_opts_find(list, target);
+    if (opts == NULL) {
+        opts = QTAILQ_FIRST(&list->head);
+        if (!opts) {
+            return g_strdup("iqn.2008-11.org.linux-kvm");
+        }
+    }
+
+    name = qemu_opt_get(opts, "initiator-name");
+    if (!name) {
+        return g_strdup("iqn.2008-11.org.linux-kvm");
+    }
+
+    return g_strdup(name);
+}
+
 /*
  * We support iscsi url's on the form
  * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
@@ -465,6 +568,7 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
     struct iscsi_context *iscsi = NULL;
     struct iscsi_url *iscsi_url = NULL;
     struct IscsiTask task;
+    char *initiator_name = NULL;
     int ret;
 
     if ((BDRV_SECTOR_SIZE % 512) != 0) {
@@ -474,16 +578,6 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
         return -EINVAL;
     }
 
-    memset(iscsilun, 0, sizeof(IscsiLun));
-
-    /* Should really append the KVM name after the ':' here */
-    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
-    if (iscsi == NULL) {
-        error_report("iSCSI: Failed to create iSCSI context.");
-        ret = -ENOMEM;
-        goto failed;
-    }
-
     iscsi_url = iscsi_parse_full_url(iscsi, filename);
     if (iscsi_url == NULL) {
         error_report("Failed to parse URL : %s %s", filename,
@@ -492,6 +586,17 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
         goto failed;
     }
 
+    memset(iscsilun, 0, sizeof(IscsiLun));
+
+    initiator_name = parse_initiator_name(iscsi_url->target);
+
+    iscsi = iscsi_create_context(initiator_name);
+    if (iscsi == NULL) {
+        error_report("iSCSI: Failed to create iSCSI context.");
+        ret = -ENOMEM;
+        goto failed;
+    }
+
     if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
         error_report("iSCSI: Failed to set target name.");
         ret = -EINVAL;
@@ -507,6 +612,14 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
             goto failed;
         }
     }
+
+    /* check if we got CHAP username/password via the options */
+    if (parse_chap(iscsi, iscsi_url->target) != 0) {
+        error_report("iSCSI: Failed to set CHAP user/password");
+        ret = -EINVAL;
+        goto failed;
+    }
+
     if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
         error_report("iSCSI: Failed to set session type to normal.");
         ret = -EINVAL;
@@ -515,6 +628,9 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
 
     iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
 
+    /* check if we got HEADER_DIGEST via the options */
+    parse_header_digest(iscsi, iscsi_url->target);
+
     task.iscsilun = iscsilun;
     task.status = 0;
     task.complete = 0;
@@ -548,6 +664,9 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
     return 0;
 
 failed:
+    if (initiator_name != NULL) {
+        g_free(initiator_name);
+    }
     if (iscsi_url != NULL) {
         iscsi_destroy_url(iscsi_url);
     }
diff --git a/qemu-config.c b/qemu-config.c
index c12c5eb..e240bd8 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -118,6 +118,32 @@ static QemuOptsList qemu_drive_opts = {
     },
 };
 
+static QemuOptsList qemu_iscsi_opts = {
+    .name = "iscsi",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_iscsi_opts.head),
+    .desc = {
+        {
+            .name = "user",
+            .type = QEMU_OPT_STRING,
+            .help = "username for CHAP authentication to target",
+        },{
+            .name = "password",
+            .type = QEMU_OPT_STRING,
+            .help = "password for CHAP authentication to target",
+        },{
+            .name = "header-digest",
+            .type = QEMU_OPT_STRING,
+            .help = "HeaderDigest setting. "
+                    "{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
+        },{
+            .name = "initiator-name",
+            .type = QEMU_OPT_STRING,
+            .help = "Initiator iqn name to use when connecting",
+        },
+        { /* end of list */ }
+    },
+};
+
 static QemuOptsList qemu_chardev_opts = {
     .name = "chardev",
     .implied_opt_name = "backend",
@@ -580,6 +606,7 @@ static QemuOptsList *vm_config_groups[32] = {
     &qemu_option_rom_opts,
     &qemu_machine_opts,
     &qemu_boot_opts,
+    &qemu_iscsi_opts,
     NULL,
 };
 
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 11f4166..83b2ad5 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -730,6 +730,57 @@ export LIBISCSI_CHAP_PASSWORD=<password>
 iscsi://<host>/<target-iqn-name>/<lun>
 @end example
 
+Various session related parameters can be set via special options, either
+in a configuration file provided via '-readconfig' or directly on the
+command line.
+
+@example
+Setting a specific initiator name to use when logging in to the target
+-iscsi initiator-name=iqn.qemu.test:my-initiator
+@end example
+
+@example
+Controlling which type of header digest to negotiate with the target
+-iscsi header-digest=CRC32C|CRC32C-NONE|NONE-CRC32C|NONE
+@end example
+
+These can also be set via a configuration file
+@example
+[iscsi]
+  user = "CHAP username"
+  password = "CHAP password"
+  initiator-name = "iqn.qemu.test:my-initiator"
+  # header digest is one of CRC32C|CRC32C-NONE|NONE-CRC32C|NONE
+  header-digest = "CRC32C"
+@end example
+
+
+Setting the target name allows different options for different targets
+@example
+[iscsi "iqn.target.name"]
+  user = "CHAP username"
+  password = "CHAP password"
+  initiator-name = "iqn.qemu.test:my-initiator"
+  # header digest is one of CRC32C|CRC32C-NONE|NONE-CRC32C|NONE
+  header-digest = "CRC32C"
+@end example
+
+
+Howto use a configuration file to set iSCSI configuration options:
+@example
+cat >iscsi.conf <<EOF
+[iscsi]
+  user = "me"
+  password = "my password"
+  initiator-name = "iqn.qemu.test:my-initiator"
+  header-digest = "CRC32C"
+EOF
+
+qemu-system-i386 -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \
+    -readconfig iscsi.conf
+@end example
+
+
 Howto set up a simple iSCSI target on loopback and accessing it via QEMU:
 @example
 This example shows how to set up an iSCSI target with one CDROM and one DISK
@@ -744,7 +795,8 @@ tgtadm --lld iscsi --mode logicalunit --op new --tid 1 --lun 2 \
     -b /IMAGES/cd.iso --device-type=cd
 tgtadm --lld iscsi --op bind --mode target --tid 1 -I ALL
 
-qemu-system-i386 -boot d -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \
+qemu-system-i386 -iscsi initiator-name=iqn.qemu.test:my-initiator \
+    -boot d -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \
     -cdrom iscsi://127.0.0.1/iqn.qemu.test/2
 @end example
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 653ff2d..d93f545 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1780,24 +1780,32 @@ Syntax for specifying iSCSI LUNs is
 
 Example (without authentication):
 @example
-qemu -cdrom iscsi://192.0.2.1/iqn.2001-04.com.example/2 \
---drive file=iscsi://192.0.2.1/iqn.2001-04.com.example/1
+qemu -iscsi initiator-name=iqn.2001-04.com.example:my-initiator \
+-cdrom iscsi://192.0.2.1/iqn.2001-04.com.example/2 \
+-drive file=iscsi://192.0.2.1/iqn.2001-04.com.example/1
 @end example
 
 Example (CHAP username/password via URL):
 @example
-qemu --drive file=iscsi://user%password@@192.0.2.1/iqn.2001-04.com.example/1
+qemu -drive file=iscsi://user%password@@192.0.2.1/iqn.2001-04.com.example/1
 @end example
 
 Example (CHAP username/password via environment variables):
 @example
 LIBISCSI_CHAP_USERNAME="user" \
 LIBISCSI_CHAP_PASSWORD="password" \
-qemu --drive file=iscsi://192.0.2.1/iqn.2001-04.com.example/1
+qemu -drive file=iscsi://192.0.2.1/iqn.2001-04.com.example/1
 @end example
 
 iSCSI support is an optional feature of QEMU and only available when
 compiled and linked against libiscsi.
+ETEXI
+DEF("iscsi", HAS_ARG, QEMU_OPTION_iscsi,
+    "-iscsi [user=user][,password=password]\n"
+    "       [,header-digest=CRC32C|CR32C-NONE|NONE-CRC32C|NONE\n"
+    "       [,initiator-name=iqn]\n"
+    "                iSCSI session parameters\n", QEMU_ARCH_ALL)
+STEXI
 
 @item NBD
 QEMU supports NBD (Network Block Devices) both using TCP protocol as well
diff --git a/vl.c b/vl.c
index d88a18c..651a2d9 100644
--- a/vl.c
+++ b/vl.c
@@ -2512,6 +2512,14 @@ int main(int argc, char **argv, char **envp)
                     exit(1);
                 }
                 break;
+#ifdef CONFIG_LIBISCSI
+            case QEMU_OPTION_iscsi:
+                opts = qemu_opts_parse(qemu_find_opts("iscsi"), optarg, 0);
+                if (!opts) {
+                    exit(1);
+                }
+                break;
+#endif
 #ifdef CONFIG_SLIRP
             case QEMU_OPTION_tftp:
                 legacy_tftp_prefix = optarg;
-- 
1.7.3.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI
  2012-01-25 22:39 ` [Qemu-devel] [PATCH] iSCSI: add " Ronnie Sahlberg
@ 2012-01-25 22:53   ` Eric Blake
  2012-01-26 10:58   ` Kevin Wolf
  1 sibling, 0 replies; 4+ messages in thread
From: Eric Blake @ 2012-01-25 22:53 UTC (permalink / raw)
  To: Ronnie Sahlberg; +Cc: kwolf, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 2252 bytes --]

On 01/25/2012 03:39 PM, Ronnie Sahlberg wrote:
> This patch adds configuration variables for iSCSI to set
> initiator-name to use when logging in to the target,
> which type of header-digest to negotiate with the target
> and username and password for CHAP authentication.
> 
> This allows specifying a initiator-name either from the command line
> -iscsi initiator-name=iqn.2004-01.com.example:test
> or from a configuration file included with -readconfig
>     [iscsi]
>       initiator-name = iqn.2004-01.com.example:test
>       header-digest = CRC32C|CRC32C-NONE|NONE-CRC32C|NONE
>       user = CHAP username
>       password = CHAP password
> 
> If you use several different targets, you can also configure this on a per
> target basis by using a group name:
>     [iscsi "iqn.target.name"]
>     ...
> 
> The configuration file can be read using -readconfig.
> Example :
> qemu-system-i386 -drive file=iscsi://127.0.0.1/iqn.ronnie.test/1
>  -readconfig iscsi.conf
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> ---
>  block/iscsi.c   |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>  qemu-config.c   |   27 +++++++++++
>  qemu-doc.texi   |   54 +++++++++++++++++++++-
>  qemu-options.hx |   16 +++++--
>  vl.c            |    8 +++
>  5 files changed, 229 insertions(+), 15 deletions(-)

I don't know the qemu code base well enough to give a full ack, but I
can at least state that you addressed my concerns from v1.  Thanks!

> +++ b/qemu-doc.texi
> @@ -730,6 +730,57 @@ export LIBISCSI_CHAP_PASSWORD=<password>
> +
> +
> +Howto use a configuration file to set iSCSI configuration options:

s/Howto/How to/

> +@example
> +cat >iscsi.conf <<EOF
> +[iscsi]
> +  user = "me"
> +  password = "my password"
> +  initiator-name = "iqn.qemu.test:my-initiator"
> +  header-digest = "CRC32C"
> +EOF
> +
> +qemu-system-i386 -drive file=iscsi://127.0.0.1/iqn.qemu.test/1 \
> +    -readconfig iscsi.conf
> +@end example
> +
> +
>  Howto set up a simple iSCSI target on loopback and accessing it via QEMU:

then again, you were copying and pasting.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI
  2012-01-25 22:39 ` [Qemu-devel] [PATCH] iSCSI: add " Ronnie Sahlberg
  2012-01-25 22:53   ` Eric Blake
@ 2012-01-26 10:58   ` Kevin Wolf
  1 sibling, 0 replies; 4+ messages in thread
From: Kevin Wolf @ 2012-01-26 10:58 UTC (permalink / raw)
  To: Ronnie Sahlberg; +Cc: eblake, qemu-devel

Am 25.01.2012 23:39, schrieb Ronnie Sahlberg:
> This patch adds configuration variables for iSCSI to set
> initiator-name to use when logging in to the target,
> which type of header-digest to negotiate with the target
> and username and password for CHAP authentication.
> 
> This allows specifying a initiator-name either from the command line
> -iscsi initiator-name=iqn.2004-01.com.example:test
> or from a configuration file included with -readconfig
>     [iscsi]
>       initiator-name = iqn.2004-01.com.example:test
>       header-digest = CRC32C|CRC32C-NONE|NONE-CRC32C|NONE
>       user = CHAP username
>       password = CHAP password
> 
> If you use several different targets, you can also configure this on a per
> target basis by using a group name:
>     [iscsi "iqn.target.name"]
>     ...
> 
> The configuration file can be read using -readconfig.
> Example :
> qemu-system-i386 -drive file=iscsi://127.0.0.1/iqn.ronnie.test/1
>  -readconfig iscsi.conf
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> ---
>  block/iscsi.c   |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++----
>  qemu-config.c   |   27 +++++++++++
>  qemu-doc.texi   |   54 +++++++++++++++++++++-
>  qemu-options.hx |   16 +++++--
>  vl.c            |    8 +++
>  5 files changed, 229 insertions(+), 15 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 938c568..bd3ca11 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -455,6 +455,109 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
>      }
>  }
>  
> +static int parse_chap(struct iscsi_context *iscsi, const char *target)
> +{
> +    QemuOptsList *list;
> +    QemuOpts *opts;
> +    const char *user = NULL;
> +    const char *password = NULL;
> +

The pattern from here...

> +    list = qemu_find_opts("iscsi");
> +    if (!list) {
> +        return 0;
> +    }
> +
> +    opts = qemu_opts_find(list, target);
> +    if (opts == NULL) {
> +        opts = QTAILQ_FIRST(&list->head);
> +        if (!opts) {
> +            return 0;
> +        }
> +    }

...to here repeats in all three parse functions. Would probably be worth
having a function for it.

> +
> +    user = qemu_opt_get(opts, "user");
> +    if (!user) {
> +        return 0;
> +    }
> +
> +    password = qemu_opt_get(opts, "password");
> +    if (!password) {
> +        error_report("CHAP username specified but no password was given");
> +        return -1;
> +    }
> +
> +    if (iscsi_set_initiator_username_pwd(iscsi, user, password)) {
> +        error_report("Failed to set initiator username and password");
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +static void parse_header_digest(struct iscsi_context *iscsi, const char *target)
> +{
> +    QemuOptsList *list;
> +    QemuOpts *opts;
> +    const char *digest = 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;
> +        }
> +    }
> +
> +    digest = qemu_opt_get(opts, "header-digest");
> +    if (!digest) {
> +        return;
> +    }
> +
> +    if (!strcmp(digest, "CRC32C")) {
> +        iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C);
> +    } else if (!strcmp(digest, "NONE")) {
> +        iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE);
> +    } else if (!strcmp(digest, "CRC32C-NONE")) {
> +        iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_CRC32C_NONE);
> +    } else if (!strcmp(digest, "NONE-CRC32C")) {
> +        iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
> +    } else {
> +        error_report("Invalid header-digest setting : %s", digest);
> +    }
> +}
> +
> +static char *parse_initiator_name(const char *target)
> +{
> +    QemuOptsList *list;
> +    QemuOpts *opts;
> +    const char *name = NULL;
> +
> +    list = qemu_find_opts("iscsi");
> +    if (!list) {
> +        return g_strdup("iqn.2008-11.org.linux-kvm");
> +    }
> +
> +    opts = qemu_opts_find(list, target);
> +    if (opts == NULL) {
> +        opts = QTAILQ_FIRST(&list->head);
> +        if (!opts) {
> +            return g_strdup("iqn.2008-11.org.linux-kvm");
> +        }
> +    }
> +
> +    name = qemu_opt_get(opts, "initiator-name");
> +    if (!name) {
> +        return g_strdup("iqn.2008-11.org.linux-kvm");
> +    }
> +
> +    return g_strdup(name);
> +}

Why do you need to strdup the strings? They look all pretty constant. An
additional advantage would be...

> +
>  /*
>   * We support iscsi url's on the form
>   * iscsi://[<username>%<password>@]<host>[:<port>]/<targetname>/<lun>
> @@ -465,6 +568,7 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
>      struct iscsi_context *iscsi = NULL;
>      struct iscsi_url *iscsi_url = NULL;
>      struct IscsiTask task;
> +    char *initiator_name = NULL;
>      int ret;
>  
>      if ((BDRV_SECTOR_SIZE % 512) != 0) {
> @@ -474,16 +578,6 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
>          return -EINVAL;
>      }
>  
> -    memset(iscsilun, 0, sizeof(IscsiLun));
> -
> -    /* Should really append the KVM name after the ':' here */
> -    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
> -    if (iscsi == NULL) {
> -        error_report("iSCSI: Failed to create iSCSI context.");
> -        ret = -ENOMEM;
> -        goto failed;
> -    }
> -
>      iscsi_url = iscsi_parse_full_url(iscsi, filename);
>      if (iscsi_url == NULL) {
>          error_report("Failed to parse URL : %s %s", filename,
> @@ -492,6 +586,17 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
>          goto failed;
>      }
>  
> +    memset(iscsilun, 0, sizeof(IscsiLun));
> +
> +    initiator_name = parse_initiator_name(iscsi_url->target);
> +
> +    iscsi = iscsi_create_context(initiator_name);
> +    if (iscsi == NULL) {
> +        error_report("iSCSI: Failed to create iSCSI context.");
> +        ret = -ENOMEM;
> +        goto failed;
> +    }
> +
>      if (iscsi_set_targetname(iscsi, iscsi_url->target)) {
>          error_report("iSCSI: Failed to set target name.");
>          ret = -EINVAL;
> @@ -507,6 +612,14 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
>              goto failed;
>          }
>      }
> +
> +    /* check if we got CHAP username/password via the options */
> +    if (parse_chap(iscsi, iscsi_url->target) != 0) {
> +        error_report("iSCSI: Failed to set CHAP user/password");
> +        ret = -EINVAL;
> +        goto failed;
> +    }
> +
>      if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
>          error_report("iSCSI: Failed to set session type to normal.");
>          ret = -EINVAL;
> @@ -515,6 +628,9 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
>  
>      iscsi_set_header_digest(iscsi, ISCSI_HEADER_DIGEST_NONE_CRC32C);
>  
> +    /* check if we got HEADER_DIGEST via the options */
> +    parse_header_digest(iscsi, iscsi_url->target);
> +
>      task.iscsilun = iscsilun;
>      task.status = 0;
>      task.complete = 0;
> @@ -548,6 +664,9 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
>      return 0;
>  
>  failed:
> +    if (initiator_name != NULL) {
> +        g_free(initiator_name);
> +    }

...that you wouldn't leak initiator_name in success case without strdup.

Kevin

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-01-26 10:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-25 22:39 [Qemu-devel] [PATCH 0/0] Add configuration variables for iSCSI Ronnie Sahlberg
2012-01-25 22:39 ` [Qemu-devel] [PATCH] iSCSI: add " Ronnie Sahlberg
2012-01-25 22:53   ` Eric Blake
2012-01-26 10:58   ` Kevin Wolf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).