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

Re: [libvirt] [PATCH 1/1] Implement NPIV



Daniel P. Berrange wrote:
On Tue, Apr 07, 2009 at 05:16:31PM -0400, David Allan wrote:
static int
+virStorageBackendVportCreateDelete(virConnectPtr conn,
+                                   virStoragePoolObjPtr pool,
+                                   int operation)
+{
+    int fd = -1;
+    int retval = 0;
+    char *operation_path;
+    const char *operation_file;
+    char *vport_name;
+    size_t towrite = 0;
+
+    switch (operation) {
+    case VPORT_CREATE:
+        operation_file = LINUX_SYSFS_VPORT_CREATE_POSTFIX;
+        break;
+    case VPORT_DELETE:
+        operation_file = LINUX_SYSFS_VPORT_DELETE_POSTFIX;
+        break;
+    default:
+        virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+                              _("Invalid vport operation (%d)"), operation);
+        retval = -1;
+        goto no_unwind;
+        break;
+    }
+
+    if (virAsprintf(&operation_path,
+                    "%s/%s/%s",
+                    LINUX_SYSFS_FC_HOST_PREFIX,
+                    pool->def->source.adapter,
+                    operation_file) < 0) {
+
+        virReportOOMError(conn);
+        retval = -1;
+        goto no_unwind;
+    }
+
+    VIR_DEBUG(_("Vport operation path is '%s'"), operation_path);
+
+    fd = open(operation_path, O_WRONLY);
+
+    if (fd < 0) {
+        virReportSystemError(conn, errno,
+                             _("Could not open '%s' for vport operation"),
+                             operation_path);
+        retval = -1;
+        goto free_path;
+    }
+
+    if (virAsprintf(&vport_name,
+                    "%s:%s",
+                    pool->def->source.wwpn,
+                    pool->def->source.wwnn) < 0) {
+
+        virReportOOMError(conn);
+        retval = -1;
+        goto close_fd;
+    }
+
+    towrite = sizeof(vport_name);
+    if (safewrite(fd, vport_name, towrite) != towrite) {
+        virReportSystemError(conn, errno,
+                             _("Write of '%s' to '%s' during "
+                               "vport create/delete failed"),
+                             vport_name, operation_path);
+        retval = -1;
+    }

This chunk of code could be a little simplified by making it
call out to virFileWriteStr() from src/util.h

Thanks for the pointer--I'll make that change.

+
+    VIR_FREE(vport_name);
+close_fd:
+    close(fd);
+free_path:
+    VIR_FREE(operation_path);
+no_unwind:
+    VIR_DEBUG("%s", _("Vport operation complete"));
+    return retval;
+}
+
+
+static int
+virStorageBackendSCSIStartPool(virConnectPtr conn,
+                               virStoragePoolObjPtr pool)
+{
+    int retval = 0;
+
+    if (pool->def->source.wwnn != NULL) {
+        retval = virStorageBackendVportCreateDelete(conn, pool, VPORT_CREATE);
+    }
+
+    return retval;
+}
+
+
+static int
+virStorageBackendSCSIStopPool(virConnectPtr conn ATTRIBUTE_UNUSED,
+                              virStoragePoolObjPtr pool ATTRIBUTE_UNUSED)
+{
+    int retval = 0;
+
+    if (pool->def->source.wwnn != NULL) {
+        retval = virStorageBackendVportCreateDelete(conn, pool, VPORT_DELETE);
+    }
+
+    return retval;
+}
+
+
+static int
 virStorageBackendSCSITriggerRescan(virConnectPtr conn,
                                    uint32_t host)
 {
     int fd = -1;
     int retval = 0;
     char *path;
+    size_t towrite = 0;
VIR_DEBUG(_("Triggering rescan of host %d"), host); @@ -593,9 +702,8 @@ virStorageBackendSCSITriggerRescan(virConnectPtr conn,
         goto free_path;
     }
- if (safewrite(fd,
-                  LINUX_SYSFS_SCSI_HOST_SCAN_STRING,
-                  sizeof(LINUX_SYSFS_SCSI_HOST_SCAN_STRING)) < 0) {
+    towrite = sizeof(LINUX_SYSFS_SCSI_HOST_SCAN_STRING);
+    if (safewrite(fd, LINUX_SYSFS_SCSI_HOST_SCAN_STRING, towrite) != towrite) {
virReportSystemError(conn, errno,
                              _("Write to '%s' to trigger host scan failed"),
@@ -645,5 +753,7 @@ out:
 virStorageBackend virStorageBackendSCSI = {
     .type = VIR_STORAGE_POOL_SCSI,
+ .startPool = virStorageBackendSCSIStartPool,
     .refreshPool = virStorageBackendSCSIRefreshPool,
+    .stopPool = virStorageBackendSCSIStopPool,

As per the previous mail, I think these are better switched over to the
.buildPool and .deletePool  drivers.

diff --git a/src/storage_conf.c b/src/storage_conf.c
index 9e25ccb..4827d69 100644
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -42,6 +42,7 @@
 #include "buf.h"
 #include "util.h"
 #include "memory.h"
+#include "logging.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE @@ -451,6 +452,55 @@ error:
 }
+static int
+getNPIVParameters(virConnectPtr conn,
+                  virStoragePoolDefPtr pool,
+                  xmlXPathContextPtr ctxt)
+{
+    int retval = 0;
+
+    if ((pool->source.wwpn = virXPathString(conn,
+                                            "string(/pool/source/adapter/@wwpn)",
+                                            ctxt)) == NULL) {
+        VIR_DEBUG("%s", _("No WWPN found"));
+        goto out;
+    }
+
+    VIR_DEBUG(_("Found WWPN '%s'"), pool->source.wwpn);
+
+    if ((pool->source.wwnn = virXPathString(conn,
+                                            "string(/pool/source/adapter/@wwnn)",
+                                            ctxt)) == NULL) {
+        VIR_DEBUG("%s", _("No WWNN found"));
+        goto out;
+    }
+
+    VIR_DEBUG(_("Found WWNN '%s'"), pool->source.wwnn);
+
+    if (pool->source.wwpn != NULL || pool->source.wwnn != NULL) {
+        if (pool->source.wwpn == NULL || pool->source.wwnn == NULL) {
+            virStorageReportError(conn, VIR_ERR_XML_ERROR,
+                                  _("Both WWPN and WWNN must be specified "
+                                    "if either is specified"));
+            retval = -1;
+            goto cleanup;
+        }
+    }
+
+    /* We don't check the values for validity, because the kernel does
+     * that.  Any invalid values will be rejected when the pool
+     * starts.  The kernel has the final say on what it will accept
+     * and we should not second guess it. */
+
+cleanup:
+    VIR_FREE(pool->source.wwpn);
+    VIR_FREE(pool->source.wwnn);
+
+out:
+    return retval;
+}
+
+
 static virStoragePoolDefPtr
 virStoragePoolDefParseDoc(virConnectPtr conn,
                           xmlXPathContextPtr ctxt,
@@ -590,6 +640,9 @@ virStoragePoolDefParseDoc(virConnectPtr conn,
                              "%s", _("missing storage pool source adapter name"));
             goto cleanup;
         }
+        if (getNPIVParameters(conn, ret, ctxt) < 0) {
+            goto cleanup;
+        }
     }
authType = virXPathString(conn, "string(/pool/source/auth/@type)", ctxt);
diff --git a/src/storage_conf.h b/src/storage_conf.h
index 4e35ccb..cfd8b14 100644
--- a/src/storage_conf.h
+++ b/src/storage_conf.h
@@ -192,6 +192,12 @@ struct _virStoragePoolSource {
     /* Or an adapter */
     char *adapter;
+ /* And an optional WWPN */
+    char *wwpn;
+
+    /* And an optional WWNN */
+    char *wwnn;
+

Since adapter, wwpn and wwnn are all associated with each other, I
think we should put them all into a union here. eg, replace the
current

      char *adapter;

with

     union {
        char *name;
        char *wwpn;
        char *wwnn;
     } adapter;

I don't have completely working code yet, but here's what I'm thinking:

We can't use a union because wwpn and wwnn will always be in use at the same time, but I agree that we're starting to collect a bunch of related information about adapters that clutters up the pool source struct. I've created a struct along the lines of virStoragePoolSourceHost to contain this information. It will need some additional fields.

/* * For adapter based pools, info on the relevant adapter
 */
typedef struct _virStoragePoolSourceAdapter virStoragePoolSourceAdapter;
typedef virStoragePoolSourceAdapter *virStoragePoolSourceAdapterPtr;
struct _virStoragePoolSourceAdapter {
    char *name;
    char *wwpn;
    char *wwnn;
};

Since we might be creating a virtual adapter on a physical adapter that we specify by wwpn/wwnn, we'll need two places to specify wwpn/wwnn in the XML. I propose:

<pool type="scsi">
  <name>npiv</name>
  <source>
    <adapter name="host6" wwpn="0000111122223333" wwnn="4444555566667777"/>
    <vport wwpn="88889999aaaabbbb" wwnn="ccccddddeeeeffff"/>
  </source>
  <target>
    <path>/dev/disk/by-id</path>
  </target>
</pool>

Dave


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