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 <ronniesahlb...@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