[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/2] Implement support for multi IQN



On Tue, Jan 19, 2010 at 10:51:21AM -0500, David Allan wrote:
> 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>
> +

  so the construct is  <iqn name="..."/>

[...]
> @@ -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);

  While this XPath expression really only capture
    <iqn>...</iqn>
Since the attribute is a good idea to preserve for future expansion, I'm
fixing this to
   "string(./initiator/iqn/@name)"

Documentation is also needed for that new construct, it's missing from
the patch.

> 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 @@
[...]
> +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);
> +    }
> +

 line is never freed so a VIR_FREE(line); is also needed here

> +    if (fp != NULL) {
> +        fclose(fp);
> +    } else {
> +        if (fd != -1) {
> +            close(fd);
> +        }
> +    }
> +
> +    return ret;
> +}
> +

> +static int
> +virStorageBackendCreateIface(virConnectPtr conn,
> +                             virStoragePoolObjPtr pool,
> +                             char **ifacename)

  I'm renaming that routine as
    virStorageBackendCreateIfaceIQN
as it should be called only if pool->def->source.initiator.iqn is non NULL

> +{
> +    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]);

  The probability of hitting an existing interface name is fairly small
but that still sounds unreliable in the long term. I would suggest as a
later improvement to check for the resulting temp_ifacename name values
and if it exists, loop before the snprintf. Not needed right now though,

> +        goto out;

  I tend to think that *ifacename should be freed and NULL'ed when
there is an error

> +    }
> +
> +    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);

  same here

> +        goto out;
> +    }
> +
> +    /* Check again to make sure the interface was created. */
> +    if (virStorageBackendIQNFound(conn, pool, ifacename) != IQN_FOUND) {

  Actually, here we leak *ifacename, as virStorageBackendIQNFound() will
just overwrite it not even read it.
  Clearly to me temp_ifacename should not be strdup'ed, and *ifacename
should be initialized only by virStorageBackendIQNFound

> +        VIR_DEBUG("Failed to find interface '%s' with IQN '%s' "
> +                  "after attempting to create it",
> +                  *ifacename, pool->def->source.initiator.iqn);

  The problem here is that we don't check that *ifacename is equal to
temp_ifacename but it's not a problem I think.

> +        goto out;
> +    } else {
> +        VIR_DEBUG("Interface '%s' with IQN '%s' was created successfully",
> +                  *ifacename, pool->def->source.initiator.iqn);
> +    }
> +
> +    ret = 0;
> +
> +out:
> +    return ret;
> +}

  So I had to seriously revamp that function w.r.t. string usage

> +
> +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));

  removed per second patch,

> +    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:

       seems ifacename is leaked here, needs a

       VIR_FREE(ifacename);

though I'm suprised it's not stored.

> +    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__ */

  So I made a number of changes, and finally commited the patch
enclosed.
  There is still need for:
    - documentation of the new option
    - I'm surprized the IQN name parsed seems never saved, I'm sure
      we dump the XML for pools somewere and that seems missing
    - and functional testing of course

 thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/
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..bd34f5e 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,8 @@ virStoragePoolDefParseSource(virConnectPtr conn,
     }
 
     source->host.name = virXPathString(conn, "string(./host/@name)", ctxt);
+    source->initiator.iqn = virXPathString(conn,
+                                       "string(./initiator/iqn/@name)", 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..5c657b4 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,254 @@ 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);
+    }
+
+    VIR_FREE(line);
+    if (fp != NULL) {
+        fclose(fp);
+    } else {
+        if (fd != -1) {
+            close(fd);
+        }
+    }
+
+    return ret;
+}
+
+
+static int
+virStorageBackendCreateIfaceIQN(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));
+
+    const char *const cmdargv1[] = {
+        ISCSIADM, "--mode", "iface", "--interface",
+        &temp_ifacename[0], "--op", "new", NULL
+    };
+
+    VIR_DEBUG("Attempting to create interface '%s' with IQN '%s'",
+              &temp_ifacename[0], 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", &temp_ifacename[0],
+        "--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",
+                  &temp_ifacename[0], 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:
+    if (ret != 0)
+        VIR_FREE(*ifacename);
+    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 (virStorageBackendCreateIfaceIQN(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
+    };
+
+    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:
+    VIR_FREE(ifacename);
+    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__ */

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]