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

Re: [libvirt] [RFC] Multi-IQN proposal



Shyam_Iyer Dell com wrote:
Would this proposal be acceptable ?

In principle, I think what you're proposing is reasonable, and is certainly contemplated by the iSCSI specs.

Example XML schema for an iSCSI storage pool created --

 <pool type="iscsi">
  <name>dell</name>
  <uuid>cf354733-b01b-8870-2040-94888640e24d</uuid>
  <capacity>0</capacity>
  <allocation>0</allocation>
  <available>0</available>
- <source>
  <initiator iqnname = "<initiator IQN1>">
  <initiator iqnname = "<initiator IQN2>">
  ........................................
  ........................................
  <host name="<iSCSI target hostname or Target IP address>" />
  <device path="<iSCSI Target IQN name>" />
  </source>
- <target>
  <path>/dev/disk/by-path</path>
- <permissions>
  <mode>0700</mode>
  <owner>0</owner>
  <group>0</group>
  </permissions>
  </target>
  </pool>

I think you have the initiator name specified in the right place in the XML. I would make the initiator iqn an element rather than an attribute, since your proposal contemplates adding additional initiator specific information later, and stylistically I think elements will be cleaner. That gives:

<initiator>
	<iqn>iqn.foo1.bar.baz</iqn>
	<iqn>iqn.foo2.bar.baz</iqn>
	<iqn>iqn.foo3.bar.baz</iqn>
</initiator>

Each initiator iqn name can be parsed to create the unique sessions.

You should propose specifically how you see the sessions being set up. Each pool currently describes something that basically resembles a session, so your proposal modifies that paradigm a bit. Another possible way to implement what you describe would be to allow zero or one initiator tags within a pool. If no initiator tag is specified, the system will use the system default; if a tag is specified, the system will attempt to use the information contained in it. The more I think about it, the more I like that approach since it keeps the pool paradigm unmodified.

This should solve the following possibilities --

* possibility of multiple IQNs for a single Guest
* option for Guest's own BIOS & initiator to use these IQNs (iSCSI in
Guest)
* option for hypervisor's initiator to use these IQNs on behalf of the
guest
(Multi-IQN)

How is this different from the first possibility?



Compile tested only. Needs beatification.

I didn't go over the code closely, but I didn't see anything that struck me as completely off base. I think it's more important to get the details of how this information will be used worked out at this point than to get the code finalized.

Dave

diff --git a/src/storage_conf.c b/src/storage_conf.c
index cb063cc..915e897 100644
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -116,6 +116,7 @@ enum {
     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),
 };


@@ -537,6 +538,33 @@ virStoragePoolDefParseXML(virConnectPtr conn,
             goto cleanup;
         }
     }
+
+/* Read the conf here */
+    if (options->flags & VIR_STORAGE_POOL_SOURCE_INITATOR_IQN) {
+       int niqn, i;
+
+       if ((niqn = virXPathNodeSet(conn, "./source/host/initiator",
ctxt, &nodeset)) < 0) {
+            virStorageReportError(conn, VIR_ERR_XML_ERROR,
+                        "%s", _("cannot extract storage pool source
devices"));
+            goto cleanup;
+        }
+       if (VIR_ALLOC_N(ret->source.niqn, niqn) < 0) {
+            VIR_FREE(nodeset);
+            virReportOOMError(conn);
+            goto cleanup;
+        }
+       for (i = 0 ; i < niqn ; i++ ) {
+           xmlChar *name = xmlGetProp(nodeset[i], BAD_CAST "iqnname");
+            if (path == NULL) {
+                VIR_FREE(nodeset);
+                virStorageReportError(conn, VIR_ERR_XML_ERROR,
+                        "%s", _("missing storage pool source device
path"));
+                goto cleanup;
+            }
+           ret->source.initiator[i].iqnname  = (char  *)name;
+       }
+    }
     if (options->flags & VIR_STORAGE_POOL_SOURCE_DEVICE) {
         xmlNodePtr *nodeset = NULL;
         int nsource, i;
diff --git a/src/storage_conf.h b/src/storage_conf.h
index 421d305..c2504be 100644
--- a/src/storage_conf.h
+++ b/src/storage_conf.h
@@ -202,7 +202,12 @@ struct _virStoragePoolSourceDevice {
     } geometry;
 };

-
+/* Initiator Attributes */
+typedef virStoragePoolSourceInitiatorAttr
*virStoragePoolSourceInitiatorAttrPtr;
+struct virStoragePoolSourceInitiatorAttr {
+    char *iqnname;
+    /* We could put other initiator specific things here */
+}

 typedef struct _virStoragePoolSource virStoragePoolSource;
 typedef virStoragePoolSource *virStoragePoolSourcePtr;
@@ -214,6 +219,9 @@ struct _virStoragePoolSource {
     int ndevice;
     virStoragePoolSourceDevicePtr devices;

+    /*And either one or more initiators*/
+    virStoragePoolSourceInitiatorAttrPtr initiator;
+
     /* Or a directory */
     char *dir;


--
Libvir-list mailing list
Libvir-list redhat com
https://www.redhat.com/mailman/listinfo/libvir-list


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