| Message ID | 1324183735-16930-2-git-send-email-ronniesahlberg@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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
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
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
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
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;
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(-)