[Qemu-devel] [PATCH] Add configuration variables for iscsi

[Qemu-devel] [PATCH] Add configuration variables for iscsi

[Ronnie Sahlberg]

Kevin, List


This is version 2 of the patch to add configuration variables for iSCSI.
This version adds 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.


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

[Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[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"]
    ...

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 block/iscsi.c   |  139 +++++++++++++++++++++++++++++++++++++++++++++++++++----
 qemu-config.c   |   27 +++++++++++
 qemu-doc.texi   |   38 +++++++++++++++-
 qemu-options.hx |   16 +++++--
 vl.c            |    8 +++
 5 files changed, 213 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 18f3020..7b59c05 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",
@@ -563,6 +589,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..cd32962 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -730,6 +730,41 @@ 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
+
+Settign 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 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 +779,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 087a3b9..338b482 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1761,24 +1761,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 d51ac2e..8660125 100644
--- a/vl.c
+++ b/vl.c
@@ -2496,6 +2496,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

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Eric Blake]

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

On 01/21/2012 03:03 AM, 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
> 

Can -readconfig support reading from an inherited fd, rather than only
taking a file name that qemu has to open()?  That way, libvirt could
create a pipe, pass in the fd with something like '-readconfig fd:nnn',
then pass in the configuration data over the pipe without ever having to
store the unencrypted CHAP password in an on-disk file (libvirt has
mechanisms already in place for storing authentication data in a secure
manner, but once libvirt has decrypted secure storage into something
that qemu can consume, writing it out to a temporary file on disk
defeats some of the security).

> +++ b/qemu-doc.texi
> @@ -730,6 +730,41 @@ 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

Can you give an actual command line that uses -readconfig, as part of
your example?

> +
> +Settign the target name allows different options for different targets

s/Settign/Setting/

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

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[ronnie sahlberg]

On Tue, Jan 24, 2012 at 5:07 AM, Eric Blake <eblake@redhat.com> wrote:
>
> Can -readconfig support reading from an inherited fd, rather than only
> taking a file name that qemu has to open()?
>
...
>
> Can you give an actual command line that uses -readconfig, as part of
> your example?
>

Thanks for the review Eric.
I will update the patch with an example using -readconfig and fix the typo.


Yes,  -readconfig can read from a pipe()  and it can also read from an
arbitrary file descriptor.

Reading from a pipe :
cat iscsi.conf | ./x86_64-softmmu/qemu-system-x86_64 -enable-kvm
-display vnc=127.0.0.1:0 -drive
file=iscsi://127.0.0.1/iqn.ronnie.test/1 -readconfig /proc/self/fd/0

Read from an arbitrary filedescriptor inherited from the parent process :
9<iscsi.conf ./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -display
vnc=127.0.0.1:0 -drive file=iscsi://127.0.0.1/iqn.ronnie.test/1
-readconfig /proc/self/fd/9


The second example shows how you can have qemu read from an arbitrary
descriptor that is inherited from the parent process (the shell in
this case)
I imagine you would pipe() then fork() and pass the read side of your
pipe to qemu here ?


If this works well or at least in some acceptable form it might be
useful for other users needing to pass sensitive config data into QEMU
too?


regards
ronnie sahlberg

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Eric Blake]

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

On 01/24/2012 11:47 PM, ronnie sahlberg wrote:
> Read from an arbitrary filedescriptor inherited from the parent process :
> 9<iscsi.conf ./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -display
> vnc=127.0.0.1:0 -drive file=iscsi://127.0.0.1/iqn.ronnie.test/1
> -readconfig /proc/self/fd/9

That requires the existence of procfs, which is not portable (although
it does work on Linux).  I'd rather see:

-readconfig fd:9

which matches things for -incoming; that is, if -readconfig starts with
'/' or '.', it is a filename; otherwise, it is a protocol:value
designation, where we recognize at least the fd: protocol where a value
is the incoming fd, but we could also recognize things like exec:
protocol which is an arbitrary command to use via popen.

> I imagine you would pipe() then fork() and pass the read side of your
> pipe to qemu here ?

Yes, the idea is that libvirt would rather pipe() and then pass the read
size fd to qemu, so that libvirt's handling of the decrypted secret
information is only ever passed over the pipe and not stored on disk.

> If this works well or at least in some acceptable form it might be
> useful for other users needing to pass sensitive config data into QEMU
> too?

Yes, the fd: notation of -incoming should be reusable in multiple
contexsts, including any other location where sensitive information must
be passed in.

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

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[ronnie sahlberg]

Fair enough.

I will send a separate tiny patch to add 'fd:<n>' support to specify
to qemu to -readconfig from a preexisting filedescriptor.

Other protocols like 'exec:' can easily be added later as needed.


regards
ronnie sahlberg

On Thu, Jan 26, 2012 at 2:57 AM, Eric Blake <eblake@redhat.com> wrote:
> On 01/24/2012 11:47 PM, ronnie sahlberg wrote:
>> Read from an arbitrary filedescriptor inherited from the parent process :
>> 9<iscsi.conf ./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -display
>> vnc=127.0.0.1:0 -drive file=iscsi://127.0.0.1/iqn.ronnie.test/1
>> -readconfig /proc/self/fd/9
>
> That requires the existence of procfs, which is not portable (although
> it does work on Linux).  I'd rather see:
>
> -readconfig fd:9
>
> which matches things for -incoming; that is, if -readconfig starts with
> '/' or '.', it is a filename; otherwise, it is a protocol:value
> designation, where we recognize at least the fd: protocol where a value
> is the incoming fd, but we could also recognize things like exec:
> protocol which is an arbitrary command to use via popen.
>
>> I imagine you would pipe() then fork() and pass the read side of your
>> pipe to qemu here ?
>
> Yes, the idea is that libvirt would rather pipe() and then pass the read
> size fd to qemu, so that libvirt's handling of the decrypted secret
> information is only ever passed over the pipe and not stored on disk.
>
>> If this works well or at least in some acceptable form it might be
>> useful for other users needing to pass sensitive config data into QEMU
>> too?
>
> Yes, the fd: notation of -incoming should be reusable in multiple
> contexsts, including any other location where sensitive information must
> be passed in.
>
> --
> Eric Blake   eblake@redhat.com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Kevin Wolf]

Am 25.01.2012 16:57, schrieb Eric Blake:
> On 01/24/2012 11:47 PM, ronnie sahlberg wrote:
>> Read from an arbitrary filedescriptor inherited from the parent process :
>> 9<iscsi.conf ./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -display
>> vnc=127.0.0.1:0 -drive file=iscsi://127.0.0.1/iqn.ronnie.test/1
>> -readconfig /proc/self/fd/9
> 
> That requires the existence of procfs, which is not portable (although
> it does work on Linux).  I'd rather see:
> 
> -readconfig fd:9
> 
> which matches things for -incoming; that is, if -readconfig starts with
> '/' or '.', it is a filename; otherwise, it is a protocol:value
> designation, where we recognize at least the fd: protocol where a value
> is the incoming fd, but we could also recognize things like exec:
> protocol which is an arbitrary command to use via popen.

Magic prefixes like this have one big problem: What if someone has a
config file called "fd:9"? We have the very same problem with protocols
in the block layer and while in the general case it's a convenient
syntax, we've come to hate it in cases where it misinterprets things.

Kevin

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[ronnie sahlberg]

Kevin,

Collissions are bad, but what about

IF ! STRNCMP (filename, "/proc/self/fd/", 14)  THEN
         fopen(filename, "r")
ELSE
        fdopen(atoi(filename+14), "r")
FI

modulo better validation for the atio() arguments.


Probability of anyone using "/proc/self/fd/" as a prefix for normal
files is very small.
Small enough it will be justifiable to say "do that and you are on your own". ?


regards
ronnie sahlberg

On Thu, Jan 26, 2012 at 8:08 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 25.01.2012 16:57, schrieb Eric Blake:
>> On 01/24/2012 11:47 PM, ronnie sahlberg wrote:
>>> Read from an arbitra filedescriptor inherited from the parent process :
>>> 9<iscsi.conf ./x86_64-softmmu/qemu-system-x86_64 -enable-kvm -display
>>> vnc=127.0.0.1:0 -drive file=iscsi://127.0.0.1/iqn.ronnie.test/1
>>> -readconfig /proc/self/fd/9
>>
>> That requires the existence of procfs, which is not portable (although
>> it does work on Linux).  I'd rather see:
>>
>> -readconfig fd:9
>>
>> which matches things for -incoming; that is, if -readconfig starts with
>> '/' or '.', it is a filename; otherwise, it is a protocol:value
>> designation, where we recognize at least the fd: protocol where a value
>> is the incoming fd, but we could also recognize things like exec:
>> protocol which is an arbitrary command to use via popen.
>
> Magic prefixes like this have one big problem: What if someone has a
> config file called "fd:9"? We have the very same problem with protocols
> in the block layer and while in the general case it's a convenient
> syntax, we've come to hate it in cases where it misinterprets things.
>
> Kevin

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Kevin Wolf]

Am 26.01.2012 10:18, schrieb ronnie sahlberg:
> Kevin,
> 
> Collissions are bad, but what about
> 
> IF ! STRNCMP (filename, "/proc/self/fd/", 14)  THEN
>          fopen(filename, "r")
> ELSE
>         fdopen(atoi(filename+14), "r")
> FI
> 
> modulo better validation for the atio() arguments.
> 
> 
> Probability of anyone using "/proc/self/fd/" as a prefix for normal
> files is very small.
> Small enough it will be justifiable to say "do that and you are on your own". ?

Interesting idea. Maybe that could work.

Kevin

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[ronnie sahlberg]

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

Ok   so what about this

You use a filename starting with "/proc/self/fd/"  and you dont have a
proc filesystem mounted? you are on your own!


regards
ronnie sahlberg



On Thu, Jan 26, 2012 at 8:27 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 26.01.2012 10:18, schrieb ronnie sahlberg:
>> Kevin,
>>
>> Collissions are bad, but what about
>>
>> IF ! STRNCMP (filename, "/proc/self/fd/", 14)  THEN
>>          fopen(filename, "r")
>> ELSE
>>         fdopen(atoi(filename+14), "r")
>> FI
>>
>> modulo better validation for the atio() arguments.
>>
>>
>> Probability of anyone using "/proc/self/fd/" as a prefix for normal
>> files is very small.
>> Small enough it will be justifiable to say "do that and you are on your own". ?
>
> Interesting idea. Maybe that could work.
>
> Kevin

[-- Attachment #2: 0001-READCONFIG-make-proc-self-fd-a-magic-prefix-to-refer.patch.gz --]
[-- Type: application/x-gzip, Size: 828 bytes --]

[-- Attachment #3: 0001-READCONFIG-make-proc-self-fd-a-magic-prefix-to-refer.patch --]
[-- Type: text/x-diff, Size: 1664 bytes --]

From 65aa496d77b839f1c48745fc5545e3e6772f724c Mon Sep 17 00:00:00 2001
From: Ronnie Sahlberg <ronniesahlberg@gmail.com>
Date: Thu, 26 Jan 2012 20:47:40 +1100
Subject: [PATCH] READCONFIG: make /proc/self/fd/ a magic prefix to refer to a specific
 filedesriptor inherited from the parent.

Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
---
 qemu-config.c   |   16 ++++++++++++++--
 qemu-options.hx |    3 ++-
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index b030205..9e709d4 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -770,8 +770,20 @@ out:
 
 int qemu_read_config_file(const char *filename)
 {
-    FILE *f = fopen(filename, "r");
-    int ret;
+    FILE *f;
+    int ret, fd;
+    char *ptr = NULL;
+
+    if (strncmp(filename, "/proc/self/fd/", 14)) {
+        f = fopen(filename, "r");
+    } else {
+        errno = 0;
+        fd = strtol(filename + 14, &ptr, 10);
+        if (errno != 0 || ptr == filename + 14 || *ptr != '\0') {
+            return -EINVAL;
+        }
+        f = fdopen(fd, "r");
+    }
 
     if (f == NULL) {
         return -errno;
diff --git a/qemu-options.hx b/qemu-options.hx
index 3a07ae8..e54af58 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2577,7 +2577,8 @@ DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
 STEXI
 @item -readconfig @var{file}
 @findex -readconfig
-Read device configuration from @var{file}.
+Read device configuration from @var{file}. Use '/proc/self/fd/<n>' as filename
+to read from an existing, inherited, filedesriptor.
 ETEXI
 DEF("writeconfig", HAS_ARG, QEMU_OPTION_writeconfig,
     "-writeconfig <file>\n"
-- 
1.7.3.1

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Michael Tokarev]

26.01.2012 13:54, ronnie sahlberg wrote:
> Ok   so what about this
>
> You use a filename starting with "/proc/self/fd/"  and you dont have a
> proc filesystem mounted? you are on your own!

No you're not:

>>> IF ! STRNCMP (filename, "/proc/self/fd/", 14)  THEN
>>>           fopen(filename, "r")
>>> ELSE
>>>          fdopen(atoi(filename+14), "r")
>>> FI

If the filename starts with /proc/self/fd/, qemu will
not try to open that file but parse the rest of the
string as a filedescriptor number.

/mjt

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Michael Tokarev]

26.01.2012 18:55, Michael Tokarev wrote:
> 26.01.2012 13:54, ronnie sahlberg wrote:
>> Ok so what about this
>>
>> You use a filename starting with "/proc/self/fd/" and you dont have a
>> proc filesystem mounted? you are on your own!

BTW, usual idiom (which was implemented in gawk for example)
is to use /dev/fd/N here, not /proc/self/fd/N which is really
linux-specific.  Linux traditionally had that symlinked to
/proc/self/fd, so it should work as is on linux too.
(I think /dev/fd/N is more widely used than /proc/self/fd --
solaris? *bsd?)

/mjt

> No you're not:
>
>>>> IF ! STRNCMP (filename, "/proc/self/fd/", 14) THEN
>>>> fopen(filename, "r")
>>>> ELSE
>>>> fdopen(atoi(filename+14), "r")
>>>> FI
>
> If the filename starts with /proc/self/fd/, qemu will
> not try to open that file but parse the rest of the
> string as a filedescriptor number.
>
> /mjt
>

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Daniel P. Berrange]

On Thu, Jan 26, 2012 at 08:54:11PM +1100, ronnie sahlberg wrote:
> Ok   so what about this
> 
> You use a filename starting with "/proc/self/fd/"  and you dont have a
> proc filesystem mounted? you are on your own!

IMHO that would be bad - turning what could easily be a platform neutral
feature into one that only works on Linux, just to avoid dealing with
command line parsing properly.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Michael Tokarev]

On 26.01.2012 13:08, Kevin Wolf wrote:
[]
>> -readconfig fd:9
> Magic prefixes like this have one big problem: What if someone has a
> config file called "fd:9"? We have the very same problem with protocols

 -readconfig ./fd:9

does the trick.

> in the block layer and while in the general case it's a convenient
> syntax, we've come to hate it in cases where it misinterprets things.

/mjt

[Qemu-devel] [PATCH 0/0] Add configuration variables for iSCSI

[Ronnie Sahlberg]

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

[Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[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

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Eric Blake]

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

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Kevin Wolf]

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

[Qemu-devel] Patch to add iSCSI configuration optionsi. Version 2

[Ronnie Sahlberg]

List,

Please find attached a patch that adds configuration variables for iSCSI to
set CHAP username/password, header-digest settings and the iqn to use when 
logging in to the target.

The patch is updated to produce an error if the username is set but no password was given.


regards
ronnie sahlberg

[Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[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

The patch also updates the manpage and qemu-doc

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

diff --git a/block/iscsi.c b/block/iscsi.c
index 938c568..92c6e79 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -455,6 +455,100 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
     }
 }
 
+static int parse_chap(struct iscsi_context *iscsi)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const char *user = NULL;
+    const char *password = NULL;
+
+    list = qemu_find_opts("iscsi");
+    if (!list) {
+        return 0;
+    }
+
+    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)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const char *digest = NULL;
+
+    list = qemu_find_opts("iscsi");
+    if (!list) {
+        return;
+    }
+
+    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(void)
+{
+    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 = 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 +559,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) {
@@ -476,8 +571,9 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
 
     memset(iscsilun, 0, sizeof(IscsiLun));
 
-    /* Should really append the KVM name after the ':' here */
-    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
+    initiator_name = parse_initiator_name();
+
+    iscsi = iscsi_create_context(initiator_name);
     if (iscsi == NULL) {
         error_report("iSCSI: Failed to create iSCSI context.");
         ret = -ENOMEM;
@@ -507,6 +603,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) != 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 +619,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);
+
     task.iscsilun = iscsilun;
     task.status = 0;
     task.complete = 0;
@@ -548,6 +655,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 18f3020..7b59c05 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",
@@ -563,6 +589,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..7b54329 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -730,6 +730,31 @@ 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
+
+
 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 +769,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 087a3b9..338b482 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1761,24 +1761,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 d51ac2e..8660125 100644
--- a/vl.c
+++ b/vl.c
@@ -2496,6 +2496,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

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Paolo Bonzini]

On 12/18/2011 05:48 AM, 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

header digest and user look like they should be options to the block 
driver.  For the password you can reuse the system used by qcow2 perhaps?

Paolo

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[ronnie sahlberg]

Hi,

You mean using the '-S' mode and setting the key after qemu has
initialized all devices but before it actually starts booting the
guest?
That would not be easy and would probably require some intrusive in
qemu itself that might affect other device types, so I am unsure.

The difference between qcow2 and iscsi and the problem is that .open()
is called for all devices before the monitor is started, so .open() is
called before we would have a chance to even hand the password to
qemu.

For qcow2 this is not a problem since even if the file is password
protected the file header is not, so you can still open the file and
read the header (to discover it is password protected) without knowing
the password.
So qcow2 can still open the file and do all the sanity checks it needs
without yet knowing the password.
You only need the password at a much later at the stage when you want
to actually start doing I/O to the device.


iSCSI is different because the username/password is not really
associated with the LUN or doing I/O, the password is required to
connect to the iscsi target itself, so without the password we can not
even connect to the target, much less run REPORTLUNS or TESTUNITREADY
to check if the LUN even exists. We cant even check if the target iqn
name even exists at all without the password.


So to pass the password similarly to how qcow2 does it, I see three
options, neither of which are especially attractive :

1, Change iscsi_open() to become a NOOP, dont even try to connect to
the target until the CPU is started (and -S has passed the password)
and do the connect on first i/o  instead of .open()
This would mean that if the target or the LUN does not even exist at
all, we wont get an early failure where QEMU would fail to start due
to "the device does not exist", instead the error would be postponed
until much later and cause qemu to error out on first i/o.
This looks like a horrible kludge.

2, Change -S so that it is called before qemu calls any of the .open()
functions for block devices.
I am not sure what impact this would have and if -S users would be
"surprised" if the monitor is open but the block devices have not yet
been scanned.
This looks difficult to easily integrate with the command line parsing anyway.
This too sounds like a horrible kludge.

3, Add a new block device method   .open_post_start() which is just
like .open()  but invoked after the CPU has been started.
So .open() is called like today before -S monitor is started, then the
new .open_post_start() would be called once -S has started the CPU
with the 'c' command.
In this case for iscsi, .open() could be changed to just create a
context for the device and establish the TCP connection to the target
but it would not log in to the target.
.open_post_start() which could be called for all block devices once
the CPU has been started would then use the context and connection
from .open()
and perform the actual login and discover the LUN.

3 would still be a little bit ugly since a whole set of error
conditions such as "target iqn does not exist", "lun does not exist",
"wrong type of LUN", "incorrect password" ...  would not be detected
until once the guest CPU is actually started.
Would be less ugly than options 1,2 though.


But even with something like option 3, to make it possible to use -S
to provide the password, this is mainly for addressing the case when
running via libvirt ? I still want to provide a mechanism for setting
the password directly in a configuration file for users that run qemu
directly from the command line and are not using -S.


comments ?


regards
ronnie sahlberg


On Mon, Dec 19, 2011 at 12:48 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 12/18/2011 05:48 AM, 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
>
>
> header digest and user look like they should be options to the block driver.
>  For the password you can reuse the system used by qcow2 perhaps?
>
> Paolo

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Paolo Bonzini]

On 12/22/2011 09:51 PM, ronnie sahlberg wrote:
> The difference between qcow2 and iscsi and the problem is that .open()
> is called for all devices before the monitor is started, so .open() is
> called before we would have a chance to even hand the password to
> qemu.
>
> For qcow2 this is not a problem since even if the file is password
> protected the file header is not, so you can still open the file and
> read the header (to discover it is password protected) without knowing
> the password.
> So qcow2 can still open the file and do all the sanity checks it needs
> without yet knowing the password.

You're right.

I see that the libvirt driver for rbd simply passes it on the command 
line.  I hope I'll be able to review the patch further today.

Paolo

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[ronnie sahlberg]

I do recognize that there is real need to provide password to iscsi
(and rbd?) via the monitor.
I myself do not use libvirt (it is just too much pain to be worth it
to try to upgrade it out-of-step with your distribution)
but I recognize that likely vast majority of users would use libvirt.


Once consensus is reached on "how to handle devices that need
two-stage setup" and what it would look like
I can volunteer to implement it. It would however be way out of scope
of and bigger than a simple iscsi-local patch


What about the idea about splitting .open() into two stages:
.open()   /* stage one, called before -S monitor is invoked */
.open_stage_2() /* called after -S finishes and when it starts to boot
the guest CPU */

?



regards
ronnie sahlberg


On Fri, Dec 23, 2011 at 8:08 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 12/22/2011 09:51 PM, ronnie sahlberg wrote:
>>
>> The difference between qcow2 and iscsi and the problem is that .open()
>> is called for all devices before the monitor is started, so .open() is
>> called before we would have a chance to even hand the password to
>> qemu.
>>
>> For qcow2 this is not a problem since even if the file is password
>> protected the file header is not, so you can still open the file and
>> read the header (to discover it is password protected) without knowing
>> the password.
>> So qcow2 can still open the file and do all the sanity checks it needs
>> without yet knowing the password.
>
>
> You're right.
>
> I see that the libvirt driver for rbd simply passes it on the command line.
>  I hope I'll be able to review the patch further today.
>
> Paolo

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Daniel P. Berrange]

On Fri, Dec 23, 2011 at 10:08:11AM +0100, Paolo Bonzini wrote:
> On 12/22/2011 09:51 PM, ronnie sahlberg wrote:
> >The difference between qcow2 and iscsi and the problem is that .open()
> >is called for all devices before the monitor is started, so .open() is
> >called before we would have a chance to even hand the password to
> >qemu.
> >
> >For qcow2 this is not a problem since even if the file is password
> >protected the file header is not, so you can still open the file and
> >read the header (to discover it is password protected) without knowing
> >the password.
> >So qcow2 can still open the file and do all the sanity checks it needs
> >without yet knowing the password.
> 
> You're right.
> 
> I see that the libvirt driver for rbd simply passes it on the
> command line.  I hope I'll be able to review the patch further
> today.

That is a temporary hack we want to remove, once QEMU provides a secure
way to specify the password. If we can't provide the password via the
monitor, then another alternative would be to pass down another open
file descriptor and simply have QEMU read the passwd from that ?

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Kevin Wolf]

Am 18.12.2011 05:48, 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
> 
> The patch also updates the manpage and qemu-doc
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>

So these options are global? What if I wanted to use two different
setups for two different images?

Kevin

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[ronnie sahlberg]

On Thu, Jan 19, 2012 at 11:17 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 18.12.2011 05:48, 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
>>
>> The patch also updates the manpage and qemu-doc
>>
>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>
> So these options are global? What if I wanted to use two different
> setups for two different images?
>


Good point.
I will rework the patch so that it first checks for
[iscsi "iqn.target.name"]
and if that is not found it falls-back to just checking for [iscsi]

That would allow to have one "catch all" section for all targets, but
also the possibility to override and use different settings on a
per-target basis.

I will post an updated patch in a day or two.



regards
ronnie sahlberg

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Kevin Wolf]

Am 20.01.2012 09:58, schrieb ronnie sahlberg:
> On Thu, Jan 19, 2012 at 11:17 PM, Kevin Wolf <kwolf@redhat.com> wrote:
>> Am 18.12.2011 05:48, 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
>>>
>>> The patch also updates the manpage and qemu-doc
>>>
>>> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
>>
>> So these options are global? What if I wanted to use two different
>> setups for two different images?
>>
> 
> 
> Good point.
> I will rework the patch so that it first checks for
> [iscsi "iqn.target.name"]
> and if that is not found it falls-back to just checking for [iscsi]
> 
> That would allow to have one "catch all" section for all targets, but
> also the possibility to override and use different settings on a
> per-target basis.
> 
> I will post an updated patch in a day or two.

Thanks, this sounds like a good solution.

Kevin

[Qemu-devel] Patch to add iSCSI configuration options

[Ronnie Sahlberg]

List,
Please find a patch that adds configuration options for iSCSI.
The patch allows setting the options either via -iscsi on the command
line or via a configuration file read by -readconfig

Options available at this stage are
* initiator-name to use when logging in to the targer
* header-digest setting to negotiate
* chap username
* chap password

Documentation for both manpage and qemu-doc are also updated

regards
ronnie sahlberg

[Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[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

The patch also updates the manpage and qemu-doc

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

diff --git a/block/iscsi.c b/block/iscsi.c
index 938c568..851b804 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -455,6 +455,97 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
     }
 }
 
+static void parse_chap(struct iscsi_context *iscsi)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const char *user = NULL;
+    const char *password = NULL;
+
+    list = qemu_find_opts("iscsi");
+    if (!list) {
+        return;
+    }
+
+    opts = QTAILQ_FIRST(&list->head);
+    if (!opts) {
+        return;
+    }
+
+    user = qemu_opt_get(opts, "user");
+    if (!user) {
+        return;
+    }
+
+    password = qemu_opt_get(opts, "password");
+    if (!password) {
+        return;
+    }
+
+    if (iscsi_set_initiator_username_pwd(iscsi, user, password)) {
+        error_report("Failed to set initiator username and password");
+        return;
+    }
+}
+
+static void parse_header_digest(struct iscsi_context *iscsi)
+{
+    QemuOptsList *list;
+    QemuOpts *opts;
+    const char *digest = NULL;
+
+    list = qemu_find_opts("iscsi");
+    if (!list) {
+        return;
+    }
+
+    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(void)
+{
+    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 = 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 +556,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) {
@@ -476,8 +568,9 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
 
     memset(iscsilun, 0, sizeof(IscsiLun));
 
-    /* Should really append the KVM name after the ':' here */
-    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
+    initiator_name = parse_initiator_name();
+
+    iscsi = iscsi_create_context(initiator_name);
     if (iscsi == NULL) {
         error_report("iSCSI: Failed to create iSCSI context.");
         ret = -ENOMEM;
@@ -507,6 +600,10 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
             goto failed;
         }
     }
+
+    /* check if we got CHAP username/password via the options */
+    parse_chap(iscsi);
+
     if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
         error_report("iSCSI: Failed to set session type to normal.");
         ret = -EINVAL;
@@ -515,6 +612,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);
+
     task.iscsilun = iscsilun;
     task.status = 0;
     task.complete = 0;
@@ -548,6 +648,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 597d7e1..8ad984d 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -90,6 +90,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",
@@ -535,6 +561,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 9c3cb62..2116085 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -731,6 +731,31 @@ 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
+
+
 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
@@ -745,7 +770,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 681eaf1..669ff42 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1753,24 +1753,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 f5afed4..0c553fa 100644
--- a/vl.c
+++ b/vl.c
@@ -2496,6 +2496,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

Re: [Qemu-devel] [PATCH] iSCSI: add configuration variables for iSCSI

[Orit Wasserman]

On 11/27/2011 01:24 AM, 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
> 
> The patch also updates the manpage and qemu-doc
> 
> Signed-off-by: Ronnie Sahlberg <ronniesahlberg@gmail.com>
> ---
>  block/iscsi.c   |  107 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qemu-config.c   |   27 ++++++++++++++
>  qemu-doc.texi   |   28 ++++++++++++++-
>  qemu-options.hx |   16 ++++++--
>  vl.c            |    8 ++++
>  5 files changed, 179 insertions(+), 7 deletions(-)
> 
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 938c568..851b804 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -455,6 +455,97 @@ iscsi_connect_cb(struct iscsi_context *iscsi, int status, void *command_data,
>      }
>  }
>  
> +static void parse_chap(struct iscsi_context *iscsi)
> +{
> +    QemuOptsList *list;
> +    QemuOpts *opts;
> +    const char *user = NULL;
> +    const char *password = NULL;
> +
> +    list = qemu_find_opts("iscsi");
> +    if (!list) {
> +        return;
> +    }
> +
> +    opts = QTAILQ_FIRST(&list->head);
> +    if (!opts) {
> +        return;
> +    }
> +
> +    user = qemu_opt_get(opts, "user");
> +    if (!user) {
> +        return;
> +    }
> +
> +    password = qemu_opt_get(opts, "password");
> +    if (!password) {

I think you should issue an error here. 
Why should a user give a user name without it's password? 

Orit

> +        return;
> +    }
> +
> +    if (iscsi_set_initiator_username_pwd(iscsi, user, password)) {
> +        error_report("Failed to set initiator username and password");
> +        return;
> +    }
> +}
> +
> +static void parse_header_digest(struct iscsi_context *iscsi)
> +{
> +    QemuOptsList *list;
> +    QemuOpts *opts;
> +    const char *digest = NULL;
> +
> +    list = qemu_find_opts("iscsi");
> +    if (!list) {
> +        return;
> +    }
> +
> +    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(void)
> +{
> +    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 = 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 +556,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) {
> @@ -476,8 +568,9 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
>  
>      memset(iscsilun, 0, sizeof(IscsiLun));
>  
> -    /* Should really append the KVM name after the ':' here */
> -    iscsi = iscsi_create_context("iqn.2008-11.org.linux-kvm:");
> +    initiator_name = parse_initiator_name();
> +
> +    iscsi = iscsi_create_context(initiator_name);
>      if (iscsi == NULL) {
>          error_report("iSCSI: Failed to create iSCSI context.");
>          ret = -ENOMEM;
> @@ -507,6 +600,10 @@ static int iscsi_open(BlockDriverState *bs, const char *filename, int flags)
>              goto failed;
>          }
>      }
> +
> +    /* check if we got CHAP username/password via the options */
> +    parse_chap(iscsi);
> +
>      if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) != 0) {
>          error_report("iSCSI: Failed to set session type to normal.");
>          ret = -EINVAL;
> @@ -515,6 +612,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);
> +
>      task.iscsilun = iscsilun;
>      task.status = 0;
>      task.complete = 0;
> @@ -548,6 +648,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 597d7e1..8ad984d 100644
> --- a/qemu-config.c
> +++ b/qemu-config.c
> @@ -90,6 +90,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",
> @@ -535,6 +561,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 9c3cb62..2116085 100644
> --- a/qemu-doc.texi
> +++ b/qemu-doc.texi
> @@ -731,6 +731,31 @@ 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
> +
> +
>  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
> @@ -745,7 +770,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 681eaf1..669ff42 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1753,24 +1753,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 f5afed4..0c553fa 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2496,6 +2496,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;