4:13 p.m.
Shyam_Iyer(a)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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list