The following series of patches are adding significant extra functionality to the iSCSI driver. THe current internal helper methods are not sufficiently flexible to cope with these changes. This patch refactors the code to avoid needing to have a virStoragePoolObjPtr instance as a parameter, instead passing individual target, portal and initiatoriqn parameters. It also removes hardcoding of port 3260 in the portal address, instead using the XML value if any. * src/storage/storage_backend_iscsi.c: Refactor internal helper methods --- src/storage/storage_backend_iscsi.c | 251 +++++++++++++++++------------------ 1 files changed, 125 insertions(+), 126 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 06a89ec..51f71af 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -89,6 +89,27 @@ virStorageBackendISCSITargetIP(const char *hostname, return 0; } +static char * +virStorageBackendISCSIPortal(virStoragePoolSourcePtr source) +{ + char ipaddr[NI_MAXHOST]; + char *portal; + + if (virStorageBackendISCSITargetIP(source->host.name, + ipaddr, sizeof(ipaddr)) < 0) + return NULL; + + if (virAsprintf(&portal, "%s:%d,1", ipaddr, + source->host.port ? + source->host.port : 3260) < 0) { + virReportOOMError(); + return NULL; + } + + return portal; +} + + static int virStorageBackendISCSIExtractSession(virStoragePoolObjPtr pool, char **const groups, @@ -156,7 +177,7 @@ virStorageBackendISCSISession(virStoragePoolObjPtr pool, #define LINE_SIZE 4096 static int -virStorageBackendIQNFound(virStoragePoolObjPtr pool, +virStorageBackendIQNFound(const char *initiatoriqn, char **ifacename) { int ret = IQN_MISSING, fd = -1; @@ -182,7 +203,7 @@ virStorageBackendIQNFound(virStoragePoolObjPtr pool, if (virExec(prog, NULL, NULL, &child, -1, &fd, NULL, VIR_EXEC_NONE) < 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to run '%s' when looking for existing interface with IQN '%s'"), - prog[0], pool->def->source.initiator.iqn); + prog[0], initiatoriqn); ret = IQN_ERROR; goto out; @@ -215,7 +236,7 @@ virStorageBackendIQNFound(virStoragePoolObjPtr pool, } iqn++; - if (STREQ(iqn, pool->def->source.initiator.iqn)) { + if (STREQ(iqn, initiatoriqn)) { token = strtok_r(line, " ", &saveptr); *ifacename = strdup(token); if (*ifacename == NULL) { @@ -246,7 +267,7 @@ out: static int -virStorageBackendCreateIfaceIQN(virStoragePoolObjPtr pool, +virStorageBackendCreateIfaceIQN(const char *initiatoriqn, char **ifacename) { int ret = -1, exitstatus = -1; @@ -267,7 +288,7 @@ virStorageBackendCreateIfaceIQN(virStoragePoolObjPtr pool, }; VIR_DEBUG("Attempting to create interface '%s' with IQN '%s'", - &temp_ifacename[0], pool->def->source.initiator.iqn); + &temp_ifacename[0], initiatoriqn); /* Note that we ignore the exitstatus. Older versions of iscsiadm * tools returned an exit status of > 0, even if they succeeded. @@ -283,7 +304,7 @@ virStorageBackendCreateIfaceIQN(virStoragePoolObjPtr pool, const char *const cmdargv2[] = { ISCSIADM, "--mode", "iface", "--interface", &temp_ifacename[0], "--op", "update", "--name", "iface.initiatorname", "--value", - pool->def->source.initiator.iqn, NULL + initiatoriqn, NULL }; /* Note that we ignore the exitstatus. Older versions of iscsiadm tools @@ -292,19 +313,19 @@ virStorageBackendCreateIfaceIQN(virStoragePoolObjPtr pool, if (virRun(cmdargv2, &exitstatus) < 0) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to run command '%s' to update iscsi interface with IQN '%s'"), - cmdargv2[0], pool->def->source.initiator.iqn); + cmdargv2[0], initiatoriqn); goto out; } /* Check again to make sure the interface was created. */ - if (virStorageBackendIQNFound(pool, ifacename) != IQN_FOUND) { + if (virStorageBackendIQNFound(initiatoriqn, ifacename) != IQN_FOUND) { VIR_DEBUG("Failed to find interface '%s' with IQN '%s' " "after attempting to create it", - &temp_ifacename[0], pool->def->source.initiator.iqn); + &temp_ifacename[0], initiatoriqn); goto out; } else { VIR_DEBUG("Interface '%s' with IQN '%s' was created successfully", - *ifacename, pool->def->source.initiator.iqn); + *ifacename, initiatoriqn); } ret = 0; @@ -316,82 +337,72 @@ out: } + static int -virStorageBackendISCSIConnectionIQN(virStoragePoolObjPtr pool, - const char *portal, - const char *action) +virStorageBackendISCSIConnection(const char *portal, + const char *initiatoriqn, + const char *target, + const char **extraargv) { int ret = -1; + const char *const baseargv[] = { + ISCSIADM, + "--mode", "node", + "--portal", portal, + "--targetname", target, + NULL + }; + int i; + int nargs = 0; char *ifacename = NULL; + const char **cmdargv; - switch (virStorageBackendIQNFound(pool, &ifacename)) { - case IQN_FOUND: - VIR_DEBUG("ifacename: '%s'", ifacename); - break; - case IQN_MISSING: - if (virStorageBackendCreateIfaceIQN(pool, &ifacename) != 0) { - goto out; - } - break; - case IQN_ERROR: - default: - goto out; - } + for (i = 0 ; baseargv[i] != NULL ; i++) + nargs++; + for (i = 0 ; extraargv[i] != NULL ; i++) + nargs++; + if (initiatoriqn) + nargs += 2; - const char *const sendtargets[] = { - ISCSIADM, "--mode", "discovery", "--type", "sendtargets", "--portal", portal, NULL - }; - if (virRun(sendtargets, NULL) < 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to run %s to get target list"), - sendtargets[0]); - goto out; + if (VIR_ALLOC_N(cmdargv, nargs+1) < 0) { + virReportOOMError(); + return -1; } - const char *const cmdargv[] = { - ISCSIADM, "--mode", "node", "--portal", portal, - "--targetname", pool->def->source.devices[0].path, "--interface", - ifacename, action, NULL - }; + nargs = 0; + for (i = 0 ; baseargv[i] != NULL ; i++) + cmdargv[nargs++] = baseargv[i]; + for (i = 0 ; extraargv[i] != NULL ; i++) + cmdargv[nargs++] = extraargv[i]; - if (virRun(cmdargv, NULL) < 0) { - virStorageReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to run command '%s' with action '%s'"), - cmdargv[0], action); - goto out; + if (initiatoriqn) { + switch (virStorageBackendIQNFound(initiatoriqn, &ifacename)) { + case IQN_FOUND: + VIR_DEBUG("ifacename: '%s'", ifacename); + break; + case IQN_MISSING: + if (virStorageBackendCreateIfaceIQN(initiatoriqn, &ifacename) != 0) { + goto cleanup; + } + break; + case IQN_ERROR: + default: + goto cleanup; + } + + cmdargv[nargs++] = "--interface"; + cmdargv[nargs++] = ifacename; } + cmdargv[nargs++] = NULL; + + if (virRun(cmdargv, NULL) < 0) + goto cleanup; ret = 0; -out: +cleanup: + VIR_FREE(cmdargv); VIR_FREE(ifacename); - return ret; -} - - -static int -virStorageBackendISCSIConnection(virStoragePoolObjPtr pool, - const char *portal, - const char *action) -{ - int ret = 0; - - if (pool->def->source.initiator.iqn != NULL) { - - ret = virStorageBackendISCSIConnectionIQN(pool, portal, action); - - } else { - - const char *const cmdargv[] = { - ISCSIADM, "--mode", "node", "--portal", portal, - "--targetname", pool->def->source.devices[0].path, action, NULL - }; - - if (virRun(cmdargv, NULL) < 0) { - ret = -1; - } - - } return ret; } @@ -441,46 +452,18 @@ virStorageBackendISCSIRescanLUNs(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED, static int -virStorageBackendISCSILogin(virStoragePoolObjPtr pool, - const char *portal) +virStorageBackendISCSIScanTargets(const char *portal) { - const char *const cmdsendtarget[] = { - ISCSIADM, "--mode", "discovery", "--type", "sendtargets", - "--portal", portal, NULL + const char *const sendtargets[] = { + ISCSIADM, "--mode", "discovery", "--type", "sendtargets", "--portal", portal, NULL }; - - if (virRun(cmdsendtarget, NULL) < 0) + if (virRun(sendtargets, NULL) < 0) { + virStorageReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to run %s to get target list"), + sendtargets[0]); return -1; - - return virStorageBackendISCSIConnection(pool, portal, "--login"); -} - -static int -virStorageBackendISCSILogout(virStoragePoolObjPtr pool, - const char *portal) -{ - return virStorageBackendISCSIConnection(pool, portal, "--logout"); -} - -static char * -virStorageBackendISCSIPortal(virStoragePoolObjPtr pool) -{ - char ipaddr[NI_MAXHOST]; - char *portal; - - if (virStorageBackendISCSITargetIP(pool->def->source.host.name, - ipaddr, sizeof(ipaddr)) < 0) - return NULL; - - if (VIR_ALLOC_N(portal, strlen(ipaddr) + 1 + 4 + 2 + 1) < 0) { - virReportOOMError(); - return NULL; } - - strcpy(portal, ipaddr); - strcat(portal, ":3260,1"); - - return portal; + return 0; } @@ -489,7 +472,9 @@ virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { char *portal = NULL; - char *session; + char *session = NULL; + int ret = -1; + const char *loginargv[] = { "--login", NULL }; if (pool->def->source.host.name == NULL) { virStorageReportError(VIR_ERR_INTERNAL_ERROR, @@ -505,17 +490,26 @@ virStorageBackendISCSIStartPool(virConnectPtr conn ATTRIBUTE_UNUSED, } if ((session = virStorageBackendISCSISession(pool, 1)) == NULL) { - if ((portal = virStorageBackendISCSIPortal(pool)) == NULL) - return -1; - if (virStorageBackendISCSILogin(pool, portal) < 0) { - VIR_FREE(portal); - return -1; - } - VIR_FREE(portal); - } else { - VIR_FREE(session); + if ((portal = virStorageBackendISCSIPortal(&pool->def->source)) == NULL) + goto cleanup; + /* + * iscsiadm doesn't let you login to a target, unless you've + * first issued a 'sendtargets' command to the portal :-( + */ + if (virStorageBackendISCSIScanTargets(portal) < 0) + goto cleanup; + + if (virStorageBackendISCSIConnection(portal, + pool->def->source.initiator.iqn, + pool->def->source.devices[0].path, + loginargv) < 0) + goto cleanup; } - return 0; + ret = 0; + +cleanup: + VIR_FREE(session); + return ret; } static int @@ -546,18 +540,23 @@ static int virStorageBackendISCSIStopPool(virConnectPtr conn ATTRIBUTE_UNUSED, virStoragePoolObjPtr pool) { + const char *logoutargv[] = { "--logout", NULL }; char *portal; + int ret = -1; - if ((portal = virStorageBackendISCSIPortal(pool)) == NULL) + if ((portal = virStorageBackendISCSIPortal(&pool->def->source)) == NULL) return -1; - if (virStorageBackendISCSILogout(pool, portal) < 0) { - VIR_FREE(portal); - return -1; - } - VIR_FREE(portal); + if (virStorageBackendISCSIConnection(portal, + pool->def->source.initiator.iqn, + pool->def->source.devices[0].path, + logoutargv) < 0) + goto cleanup; + ret = 0; - return 0; +cleanup: + VIR_FREE(portal); + return ret; } virStorageBackend virStorageBackendISCSI = { -- 1.7.2.3 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list