In other words, permit the initiator to use a variety of IQNs rather than just the system IQN when creating iSCSI pools. --- docs/schemas/storagepool.rng | 12 ++ src/conf/storage_conf.c | 16 ++- src/conf/storage_conf.h | 9 ++ src/storage/storage_backend_iscsi.c | 253 +++++++++++++++++++++++++++++++++- src/storage/storage_backend_iscsi.h | 4 + 5 files changed, 280 insertions(+), 14 deletions(-) diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 249bf9c..247664e 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -188,6 +188,15 @@ </element> </define> + <define name='initiatorinfoiqn'> + <element name='iqn'> + <attribute name='name'> + <text/> + </attribute> + <empty/> + </element> + </define> + <define name='devextents'> <oneOrMore> <element name='freeExtent'> @@ -363,6 +372,9 @@ <ref name='sourceinfohost'/> <ref name='sourceinfodev'/> <optional> + <ref name='initiatorinfoiqn'/> + </optional> + <optional> <ref name='sourceinfoauth'/> </optional> </element> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ea49531..f56abdd 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -106,11 +106,12 @@ struct _virStorageVolOptions { /* Flags to indicate mandatory components in the pool source */ enum { - VIR_STORAGE_POOL_SOURCE_HOST = (1<<0), - VIR_STORAGE_POOL_SOURCE_DEVICE = (1<<1), - VIR_STORAGE_POOL_SOURCE_DIR = (1<<2), - VIR_STORAGE_POOL_SOURCE_ADAPTER = (1<<3), - VIR_STORAGE_POOL_SOURCE_NAME = (1<<4), + VIR_STORAGE_POOL_SOURCE_HOST = (1<<0), + VIR_STORAGE_POOL_SOURCE_DEVICE = (1<<1), + VIR_STORAGE_POOL_SOURCE_DIR = (1<<2), + VIR_STORAGE_POOL_SOURCE_ADAPTER = (1<<3), + VIR_STORAGE_POOL_SOURCE_NAME = (1<<4), + VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN = (1<<5), }; @@ -179,7 +180,8 @@ static virStoragePoolTypeInfo poolTypeInfo[] = { { .poolType = VIR_STORAGE_POOL_ISCSI, .poolOptions = { .flags = (VIR_STORAGE_POOL_SOURCE_HOST | - VIR_STORAGE_POOL_SOURCE_DEVICE), + VIR_STORAGE_POOL_SOURCE_DEVICE | + VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN), }, .volOptions = { .formatToString = virStoragePoolFormatDiskTypeToString, @@ -283,6 +285,7 @@ virStoragePoolSourceFree(virStoragePoolSourcePtr source) { VIR_FREE(source->dir); VIR_FREE(source->name); VIR_FREE(source->adapter); + VIR_FREE(source->initiator.iqn); if (source->authType == VIR_STORAGE_POOL_AUTH_CHAP) { VIR_FREE(source->auth.chap.login); @@ -421,6 +424,7 @@ virStoragePoolDefParseSource(virConnectPtr conn, } source->host.name = virXPathString(conn, "string(./host/@name)", ctxt); + source->initiator.iqn = virXPathString(conn, "string(./initiator/iqn)", ctxt); nsource = virXPathNodeSet(conn, "./device", ctxt, &nodeset); if (nsource > 0) { diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index a795981..a5f0100 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -182,6 +182,12 @@ struct _virStoragePoolSourceDeviceExtent { int type; /* free space type */ }; +typedef struct _virStoragePoolSourceInitiatorAttr virStoragePoolSourceInitiatorAttr; +struct _virStoragePoolSourceInitiatorAttr { + /* Initiator IQN */ + char *iqn; +}; + /* * Pools can be backed by one or more devices, and some * allow us to track free space on underlying devices. @@ -223,6 +229,9 @@ struct _virStoragePoolSource { /* Or a name */ char *name; + /* Initiator IQN */ + virStoragePoolSourceInitiatorAttr initiator; + int authType; /* virStoragePoolAuthType */ union { virStoragePoolAuthChap chap; diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index b516add..e0788cf 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -33,16 +33,17 @@ #include <fcntl.h> #include <unistd.h> #include <dirent.h> +#include <sys/stat.h> #include "virterror_internal.h" #include "storage_backend_scsi.h" #include "storage_backend_iscsi.h" #include "util.h" #include "memory.h" +#include "logging.h" #define VIR_FROM_THIS VIR_FROM_STORAGE - static int virStorageBackendISCSITargetIP(virConnectPtr conn, const char *hostname, @@ -153,21 +154,257 @@ virStorageBackendISCSISession(virConnectPtr conn, return session; } + +#define LINE_SIZE 4096 + +static int +virStorageBackendIQNFound(virConnectPtr conn, + virStoragePoolObjPtr pool, + char **ifacename) +{ + int ret = IQN_MISSING, fd = -1; + char ebuf[64]; + FILE *fp = NULL; + pid_t child = 0; + char *line = NULL, *newline = NULL, *iqn = NULL, *token = NULL, + *saveptr = NULL; + const char *const prog[] = { + ISCSIADM, "--mode", "iface", NULL + }; + + if (VIR_ALLOC_N(line, LINE_SIZE) != 0) { + ret = IQN_ERROR; + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Could not allocate memory for output of '%s'"), + prog[0]); + goto out; + } + + memset(line, 0, LINE_SIZE); + + if (virExec(conn, prog, NULL, NULL, &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to run '%s' when looking for existing interface with IQN '%s'"), + prog[0], pool->def->source.initiator.iqn); + + ret = IQN_ERROR; + goto out; + } + + if ((fp = fdopen(fd, "r")) == NULL) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to open stream for file descriptor " + "when reading output from '%s': '%s'"), + prog[0], virStrerror(errno, ebuf, sizeof ebuf)); + ret = IQN_ERROR; + goto out; + } + + while (fgets(line, LINE_SIZE, fp) != NULL) { + newline = strrchr(line, '\n'); + if (newline == NULL) { + ret = IQN_ERROR; + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Unexpected line > %d characters " + "when parsing output of '%s'"), + LINE_SIZE, prog[0]); + goto out; + } + *newline = '\0'; + + iqn = strrchr(line, ','); + if (iqn == NULL) { + continue; + } + iqn++; + + if (STREQ(iqn, pool->def->source.initiator.iqn)) { + token = strtok_r(line, " ", &saveptr); + *ifacename = strdup(token); + if (*ifacename == NULL) { + ret = IQN_ERROR; + virReportOOMError(conn); + goto out; + } + VIR_DEBUG("Found interface '%s' with IQN '%s'", *ifacename, iqn); + ret = IQN_FOUND; + break; + } + } + +out: + if (ret == IQN_MISSING) { + VIR_DEBUG("Could not find interface witn IQN '%s'", iqn); + } + + if (fp != NULL) { + fclose(fp); + } else { + if (fd != -1) { + close(fd); + } + } + + return ret; +} + + +static int +virStorageBackendCreateIface(virConnectPtr conn, + virStoragePoolObjPtr pool, + char **ifacename) +{ + int ret = -1, exitstatus = -1; + char temp_ifacename[32]; + + if (virRandomInitialize(time(NULL) ^ getpid()) == -1) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to initialize random generator " + "when creating iscsi interface")); + goto out; + } + + snprintf(temp_ifacename, sizeof(temp_ifacename), "libvirt-iface-%08x", virRandom(1024 * 1024 * 1024)); + *ifacename = strdup(temp_ifacename); + if (*ifacename == NULL) { + virReportOOMError(conn); + goto out; + } + + const char *const cmdargv1[] = { + ISCSIADM, "--mode", "iface", "--interface", + *ifacename, "--op", "new", NULL + }; + + VIR_DEBUG("Attempting to create interface '%s' with IQN '%s'", + *ifacename, pool->def->source.initiator.iqn); + + /* Note that we ignore the exitstatus. Older versions of iscsiadm + * tools returned an exit status of > 0, even if they succeeded. + * We will just rely on whether the interface got created + * properly. */ + if (virRun(conn, cmdargv1, &exitstatus) < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to run command '%s' to create new iscsi interface"), + cmdargv1[0]); + goto out; + } + + const char *const cmdargv2[] = { + ISCSIADM, "--mode", "iface", "--interface", *ifacename, + "--op", "update", "--name", "iface.initiatorname", "--value", + pool->def->source.initiator.iqn, NULL + }; + + /* Note that we ignore the exitstatus. Older versions of iscsiadm tools + * returned an exit status of > 0, even if they succeeded. We will just + * rely on whether iface file got updated properly. */ + if (virRun(conn, cmdargv2, &exitstatus) < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to run command '%s' to update iscsi interface with IQN '%s'"), + cmdargv1[0], pool->def->source.initiator.iqn); + goto out; + } + + /* Check again to make sure the interface was created. */ + if (virStorageBackendIQNFound(conn, pool, ifacename) != IQN_FOUND) { + VIR_DEBUG("Failed to find interface '%s' with IQN '%s' " + "after attempting to create it", + *ifacename, pool->def->source.initiator.iqn); + goto out; + } else { + VIR_DEBUG("Interface '%s' with IQN '%s' was created successfully", + *ifacename, pool->def->source.initiator.iqn); + } + + ret = 0; + +out: + return ret; +} + + +static int +virStorageBackendISCSIConnectionIQN(virConnectPtr conn, + virStoragePoolObjPtr pool, + const char *portal, + const char *action) +{ + int ret = -1; + char *ifacename = NULL; + + switch (virStorageBackendIQNFound(conn, pool, &ifacename)) { + case IQN_FOUND: + VIR_DEBUG("ifacename: '%s'", ifacename); + break; + case IQN_MISSING: + if (virStorageBackendCreateIface(conn, pool, &ifacename) != 0) { + goto out; + } + break; + case IQN_ERROR: + default: + goto out; + } + + const char *const sendtargets[] = { + ISCSIADM, "--mode", "discovery", "--type", "sendtargets", "--portal", portal, NULL + }; + if (virRun(conn, sendtargets, NULL) < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to run %s to get target list"), + sendtargets[0]); + goto out; + } + + const char *const cmdargv[] = { + ISCSIADM, "--mode", "node", "--portal", portal, + "--targetname", pool->def->source.devices[0].path, "--interface", + ifacename, action, NULL + }; + + VIR_DEBUG("cmdargv: '%s'", virArgvToString(cmdargv)); + + if (virRun(conn, cmdargv, NULL) < 0) { + virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, + _("Failed to run command '%s' with action '%s'"), + cmdargv[0], action); + goto out; + } + + ret = 0; + +out: + return ret; +} + + static int virStorageBackendISCSIConnection(virConnectPtr conn, virStoragePoolObjPtr pool, const char *portal, const char *action) { - const char *const cmdargv[] = { - ISCSIADM, "--mode", "node", "--portal", portal, - "--targetname", pool->def->source.devices[0].path, action, NULL - }; + int ret = 0; - if (virRun(conn, cmdargv, NULL) < 0) - return -1; + if (pool->def->source.initiator.iqn != NULL) { - return 0; + ret = virStorageBackendISCSIConnectionIQN(conn, pool, portal, action); + + } else { + + const char *const cmdargv[] = { + ISCSIADM, "--mode", "node", "--portal", portal, + "--targetname", pool->def->source.devices[0].path, action, NULL + }; + + if (virRun(conn, cmdargv, NULL) < 0) { + ret = -1; + } + + } + + return ret; } diff --git a/src/storage/storage_backend_iscsi.h b/src/storage/storage_backend_iscsi.h index 665ed13..8878342 100644 --- a/src/storage/storage_backend_iscsi.h +++ b/src/storage/storage_backend_iscsi.h @@ -28,4 +28,8 @@ extern virStorageBackend virStorageBackendISCSI; +#define IQN_FOUND 1 +#define IQN_MISSING 0 +#define IQN_ERROR -1 + #endif /* __VIR_STORAGE_BACKEND_ISCSI_H__ */ -- 1.6.5.5

--- build-aux/.gitignore | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/build-aux/.gitignore b/build-aux/.gitignore index af7a347..043aafa 100644 --- a/build-aux/.gitignore +++ b/build-aux/.gitignore @@ -5,3 +5,4 @@ /gitlog-to-changelog /useless-if-before-free /vc-list-files +/warn-on-use.h -- 1.6.5.5

On 01/21/2010 06:56 AM, Daniel Veillard wrote: Yes, completely unrelated. I remember someone (Jim maybe?) saying very recently that changes to gitignore should be committed, so I included it. Perhaps my memory is faulty, though. Dave