[libvirt] Re: iSCSI Multi-IQN (Libvirt Support)
Dave Allan
dallan at redhat.com
Thu Oct 22 18:35:46 UTC 2009
Shyam_Iyer at Dell.com wrote:
> The following patch set realizes the multi-IQN concept discussed in an
> earlier thread
> http://www.mail-archive.com/libvir-list@redhat.com/msg16706.html
>
> The patch realizes an XML schema like the one below and allows libvirt
> to read through it to create storage pools.
> These XMLs when created using a virtualization manager realize unique VM
> to storage LUN mappings through a single console and thus opening up
> possibilities for the following scenarios -
>
> * possibility of multiple IQNs for a single Guest
> * option for hypervisor's initiator to use these IQNs on behalf of the
> guest
>
> Example Usages:
> Usage 1:
> VM1 - > <Init iqn1> <------------------------> <Target iqn1>
> <Init iqn2> <------------------------> <Target iqn1>
> <Init iqn3> <------------------------> <Target iqn1>
> <Init iqn4> <------------------------> <Target iqn1>
>
> Usage 2:
> VM1 - > <Init iqn1> <------------------------> <Target iqn1>
> <Init iqn2> <------------------------> <Target iqn2>
> <Init iqn3> <------------------------> <Target iqn3>
> <Init iqn4> <------------------------> <Target iqn4>
>
> Usage 3:
> VM1 - > <Init iqn1> <------------------------> <Target iqn1>
>
> VM2 - > <Init iqn2> <------------------------> <Target iqn1>
>
> Usage 4:
> VM1 - > <Init iqn1> <------------------------> <Target iqn1>
>
> VM2 - > <Init iqn2> <------------------------> <Target iqn2>
>
> 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>">
> iqnname = "<initiator IQN2>">
> </initiator>
> ........................................
> ........................................
> <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>
>
> Each initiator iqn name can be parsed to create the unique sessions.
>
> TODO:
> 1) Virt Manager GUI support to allow a visual mapping of iqn->storage
> pool -> VM and thus creating the XML for consumption by libvirt.
> Libvirt support added above can realize the same using virsh options.
>
> 2) option for Guest's own BIOS & initiator to use the initiator IQNs
> (iSCSI in Guest)
> a) Libvirt support to allow iqn as an attribute for a VM's XML.
> b) Qemu Support to allow Guest's bios & initiator to use these
> IQNs.
>
>
> Signed-off-by: Sudhir Bellad <sudhir_bellad at dell.com>
> Signed-off-by: Shyam Iyer <shyam_iyer at dell.com>
>
> diff --git a/src/storage_backend_iscsi.c b/src/storage_backend_iscsi.c
> index b516add..3f2a79d 100644
> --- a/src/storage_backend_iscsi.c
> +++ b/src/storage_backend_iscsi.c
> @@ -39,6 +39,10 @@
> #include "storage_backend_iscsi.h"
> #include "util.h"
> #include "memory.h"
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +#include <unistd.h>
>
> #define VIR_FROM_THIS VIR_FROM_STORAGE
>
> @@ -159,13 +163,57 @@ virStorageBackendISCSIConnection(virConnectPtr
> conn,
> const char *portal,
> const char *action)
> {
> - const char *const cmdargv[] = {
> - ISCSIADM, "--mode", "node", "--portal", portal,
> - "--targetname", pool->def->source.devices[0].path, action, NULL
> - };
> -
> - if (virRun(conn, cmdargv, NULL) < 0)
> - return -1;
> + DIR *dir;
> + struct dirent *entry;
> +
> +
> + if (pool->def->source.initiator[0].iqnname != NULL) {
> + int i = 0;
> + while(pool->def->source.initiator[i].iqnname != NULL){
> + if (!(dir = opendir(IFACES_DIR))) {
> + if (errno == ENOENT)
> + return 0;
> + virReportSystemError(conn, errno,
> _("Failed to open dir '%s'"),
> + IFACES_DIR);
> + return -1;
> + }
> + while ((entry = readdir(dir))) {
> + FILE *fp;
> + char path[PATH_MAX];
> +
> + if (entry->d_name[0] == '.')
> + continue;
> +
> + sprintf(path,"%s", IFACES_DIR);
> + strcat(path,(const char
> *)entry->d_name);
> +
> + if ((fp = fopen(path, "r"))){
> + char buff[256];
> + while (fp != NULL && fgets(buff,
> sizeof(buff), fp) != NULL) {
> + if (strstr(buff,
> pool->def->source.initiator[i].iqnname) != NULL) {
> + const char
> *const cmdargv[] = {
> +
> ISCSIADM, "--mode", "node", "--portal", portal,
> +
> "--targetname", pool->def->source.devices[0].path, "-I", entry->d_name,
> action, NULL
> + };
> +
> +
> if (virRun(conn, cmdargv, NULL) < 0)
> +
> return -1;
> + }
> + }
> + }
> + fclose(fp);
> + }
> + i++;
> + }
> + }
> + 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)
> + return -1;
> + }
>
> return 0;
> }
> diff --git a/src/storage_backend_iscsi.h b/src/storage_backend_iscsi.h
> index 665ed13..14c2c5c 100644
> --- a/src/storage_backend_iscsi.h
> +++ b/src/storage_backend_iscsi.h
> @@ -25,7 +25,7 @@
> #define __VIR_STORAGE_BACKEND_ISCSI_H__
>
> #include "storage_backend.h"
> -
> extern virStorageBackend virStorageBackendISCSI;
> +#define IFACES_DIR "/var/lib/iscsi/ifaces/"
>
> #endif /* __VIR_STORAGE_BACKEND_ISCSI_H__ */
> diff --git a/src/storage_conf.c b/src/storage_conf.c
> index 788de15..0f2ace9 100644
> --- a/src/storage_conf.c
> +++ b/src/storage_conf.c
> @@ -106,12 +106,13 @@ 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,
> @@ -532,6 +534,34 @@ virStoragePoolDefParseXML(virConnectPtr conn,
> goto cleanup;
> }
> }
> +
> + if(options->flags & VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN) {
> + xmlNodePtr *nodeset = NULL;
> + int niqn, i;
> +
> + if((niqn = virXPathNodeSet(conn, "./initiator/iqnname", ctxt,
> &nodeset)) < 0) {
> + virStorageReportError(conn, VIR_ERR_XML_ERROR,
> + "%s", _("cannot extract storage pool source
> devices"));
> + goto cleanup;
> + }
> + if (VIR_ALLOC_N(ret->source.initiator, niqn) < 0) {
> + VIR_FREE(nodeset);
> + virReportOOMError(conn);
> + goto cleanup;
> + }
> + for (i = 0 ; i < niqn ; i++) {
> + xmlChar *name = xmlGetProp(nodeset[i], BAD_CAST "name");
> + if (name == 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;
> + }
> + VIR_FREE(nodeset);
> + }
> +
> 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 9fedb12..c77d6ae 100644
> --- a/src/storage_conf.h
> +++ b/src/storage_conf.h
> @@ -182,6 +182,13 @@ struct _virStoragePoolSourceDeviceExtent {
> int type; /* free space type */
> };
>
> +typedef struct _virStoragePoolSourceInitiatorAttr
> virStoragePoolSourceInitiatorAttr;
> +typedef virStoragePoolSourceInitiatorAttr
> *virStoragePoolSourceInitiatorAttrPtr;
> +struct _virStoragePoolSourceInitiatorAttr {
> + /* Initiator iqn name */
> + char *iqnname;
> +};
> +
> /*
> * Pools can be backed by one or more devices, and some
> * allow us to track free space on underlying devices.
> @@ -223,6 +230,9 @@ struct _virStoragePoolSource {
> /* Or a name */
> char *name;
>
> + /* One or more initiator names */
> + virStoragePoolSourceInitiatorAttrPtr initiator;
> +
> int authType; /* virStoragePoolAuthType */
> union {
> virStoragePoolAuthChap chap;
>
I like the functionality. To restate what I said when you first
proposed it, though, each pool is currently based on one iSCSI session,
so to preserve that paradigm, you should only allow zero or one
initiator IQNs per pool. If the pool contains an initiator IQN, it will
be used when creating the session. If it does not contain an initiator
IQN, the system default initiator IQN will be used. If you require
multiple initiator IQNs, you create multiple pools, one per initiator
IQN each with the same target. I think that approach will result in a
very small patch. Do you have a specific use case for which that
approach would not work?
Dave
More information about the libvir-list
mailing list