2012/11/10 Ata E Husain Bohra <ata.husain@xxxxxxxxxxx>: > The patch adds the backend driver to support iSCSI format storage pools > and volumes for ESX host. The mapping of ESX iSCSI specifics to Libvirt > is as follows: > > 1. ESX static iSCSI target <------> Libvirt Storage Pools > 2. ESX iSCSI LUNs <------> Libvirt Storage Volumes. > > The above understanding is based on http://libvirt.org/storage.html. > > The operation supported on iSCSI pools includes: > > 1. List storage pools & volumes. > 2. Get xml descriptor operaion on pools & volumes. > 3. Lookup operation on pools & volumes by name, uuid and path (if applicable). > > iSCSI pools does not support operations such as: Create / remove pools > and volumes. > --- > src/Makefile.am | 1 + > src/esx/esx_storage_backend_iscsi.c | 807 +++++++++++++++++++++++++++++++++++ > src/esx/esx_storage_backend_iscsi.h | 29 ++ > src/esx/esx_storage_driver.c | 8 +- > src/esx/esx_vi.c | 332 ++++++++++++++ > src/esx/esx_vi.h | 18 + > src/esx/esx_vi_generator.input | 302 +++++++++++++ > src/esx/esx_vi_generator.py | 19 + > 8 files changed, 1515 insertions(+), 1 deletion(-) > create mode 100644 src/esx/esx_storage_backend_iscsi.c > create mode 100644 src/esx/esx_storage_backend_iscsi.h > > --- /dev/null > +++ b/src/esx/esx_storage_backend_iscsi.c > +static int > +esxStorageBackendISCSIPoolListStorageVolumes(virStoragePoolPtr pool, > + char **const names, > + int maxnames) > +{ > + int count = 0; > + esxPrivate *priv = pool->conn->storagePrivateData; > + esxVI_HostScsiTopologyLun *hostScsiTopologyLunList = NULL; > + const esxVI_HostScsiTopologyLun *hostScsiTopologyLun = NULL; > + esxVI_ScsiLun *scsiLunList = NULL; > + const esxVI_ScsiLun *scsiLun = NULL; > + bool success = false; > + int i = 0; > + > + if (esxVI_LookupHostScsiTopologyLunListByTargetName( > + priv->primary, pool->name, &hostScsiTopologyLunList) < 0) { > + goto cleanup; > + } > + > + if (hostScsiTopologyLunList == NULL) { > + /* iSCSI adapter may not be enabled on ESX host */ > + return 0; > + } > + > + if (esxVI_LookupScsiLunList(priv->primary, &scsiLunList) < 0) { > + goto cleanup; > + } > + > + /* O^2 but still faster than hash given N is not that large */ This comment is wrong here, because a hash wouldn't help here. In order to get the storage volume list this 2-dimensional list need to be iterated. There is no way to do this faster, except adding a cache but then you get the problem of ehen to invalidate it. So this 2 level for loop is totally fine and the comment should be removed, because it is misleading. > + for (scsiLun = scsiLunList; scsiLun != NULL && count < maxnames; > + scsiLun = scsiLun->_next) { > + for (hostScsiTopologyLun = hostScsiTopologyLunList; > + hostScsiTopologyLun != NULL && count < maxnames; > + hostScsiTopologyLun = hostScsiTopologyLun->_next) { > + if (STREQ(hostScsiTopologyLun->scsiLun, scsiLun->key)) { > + names[count] = strdup(scsiLun->deviceName); > + > + if (names[count] == NULL) { > + virReportOOMError(); > + goto cleanup; > + } > + ++count; > + } > + } > + } > + > + success = true; > + > + cleanup: > + if (! success) { > + for (i = 0; i < count; ++i) { > + VIR_FREE(names[i]); > + } > + count = -1; > + } > + > + esxVI_HostScsiTopologyLun_Free(&hostScsiTopologyLunList); > + esxVI_ScsiLun_Free(&scsiLunList); > + > + return count; > +} > +static int > +esxStorageBackendISCSIPoolRefresh(virStoragePoolPtr pool, > + unsigned int flags) > +{ > + int result = -1; > + esxPrivate *priv = pool->conn->storagePrivateData; > + esxVI_ManagedObjectReference *hostStorageSystem = NULL; > + esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL; > + esxVI_ObjectContent *hostSystem = NULL; > + esxVI_String *propertyNameList = NULL; > + > + virCheckFlags(0, -1); > + > + if (esxVI_String_AppendValueToList(&propertyNameList, > + "configManager.storageSystem\0") < 0 || > + esxVI_LookupHostSystemProperties(priv->primary, > + propertyNameList, &hostSystem) < 0 || > + esxVI_GetManagedObjectReference(hostSystem, > + "configManager.storageSystem", &hostStorageSystem, > + esxVI_Occurrence_RequiredItem) < 0 || No need to look up the hostStorageSystem here. It is already available here: ctx->hostSystem->configManager->storageSystem > + esxVI_LookupHostInternetScsiHba( > + priv->primary, &hostInternetScsiHba) < 0) { > + goto cleanup; > + } > + > + /** > + * ESX does not allow rescan on a particular target, > + * rescan all the static targets > + */ > + if (esxVI_RescanHba(priv->primary, hostStorageSystem, > + hostInternetScsiHba->device) < 0) { > + goto cleanup; > + } > + > + result = 0; > + > + cleanup: > + esxVI_String_Free(&propertyNameList); > + esxVI_ManagedObjectReference_Free(&hostStorageSystem); > + esxVI_ObjectContent_Free(&hostSystem); > + esxVI_HostInternetScsiHba_Free(&hostInternetScsiHba); > + > + return result; > + > +} > +static virStorageVolPtr > +esxStorageBackendISCSIVolumeCreateXML(virStoragePoolPtr pool ATTRIBUTE_UNUSED, > + const char *xmldesc ATTRIBUTE_UNUSED, > + unsigned int flags) > +{ > + virCheckFlags(0, NULL); A VIR_ERR_NO_SUPPORT error should be reported here. > + /* not supported operation for iSCSI pools */ > + return NULL; > +} > + > + > + > +static virStorageVolPtr > +esxStorageBackendISCSIVolumeCreateXMLFrom( > + virStoragePoolPtr pool ATTRIBUTE_UNUSED, > + const char *xmldesc ATTRIBUTE_UNUSED, > + virStorageVolPtr sourceVolume ATTRIBUTE_UNUSED, > + unsigned int flags) > +{ > + virCheckFlags(0, NULL); A VIR_ERR_NO_SUPPORT error should be reported here. > + /* not supported operation for iSCSI pools */ > + return NULL; > +} > +static int > +esxStorageBackendISCSIVolumeDelete(virStorageVolPtr volume ATTRIBUTE_UNUSED, > + unsigned int flags) > +{ > + virCheckFlags(0, -1); A VIR_ERR_NO_SUPPORT error should be reported here. > + /* unsupported operation for iSCSI volume */ > + return 1; Should return -1; > +} > + > + > + > +static int > +esxStorageBackendISCSIVolumeWipe(virStorageVolPtr volume ATTRIBUTE_UNUSED, > + unsigned int flags) > +{ > + virCheckFlags(0, -1); A VIR_ERR_NO_SUPPORT error should be reported here. > + /* unsupported operation for iSCSI volume */ > + return 1; Should return -1; > +} > + > + > + > +static char* > +esxStorageBackendISCSIVolumeGetPath(virStorageVolPtr volume) > +{ > + char *path; > + > + if (virAsprintf(&path, "%s", volume->name) < 0) { > + virReportOOMError(); > + return NULL; > + } Can be simplified to strdup. > + return path; > + > +} > diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c > index 9fb2c11..12100d7 100644 > --- a/src/esx/esx_vi.c > +++ b/src/esx/esx_vi.c > +int > +esxVI_LookupScsiLunList(esxVI_Context *ctx, > + esxVI_ScsiLun **ret) > +{ > + int result = -1; > + esxVI_DynamicProperty *dynamicProperty = NULL; > + esxVI_ObjectContent *hostSystem = NULL; > + esxVI_String *propertyNameList = NULL; > + esxVI_ScsiLun *scsiLunList = NULL; > + > + if (esxVI_String_AppendValueToList(&propertyNameList, > + "config.storageDevice.scsiLun\0") < 0 || > + esxVI_LookupHostSystemProperties( > + ctx, propertyNameList, &hostSystem) < 0) { > + goto cleanup; > + } > + > + for (dynamicProperty = hostSystem->propSet; > + dynamicProperty != NULL; > + dynamicProperty = dynamicProperty->_next) { > + if (STREQ(dynamicProperty->name, > + "config.storageDevice.scsiLun")) { > + if (esxVI_ScsiLun_CastListFromAnyType(dynamicProperty->val, > + &scsiLunList) < 0) { > + goto cleanup; > + } > + } else { > + VIR_WARN("Unexpected '%s' property", dynamicProperty->name); > + } > + } > + > + if (scsiLunList == NULL || > + esxVI_ScsiLun_DeepCopyList(ret, scsiLunList) < 0) { > + goto cleanup; > + } > + > + result = 0; > + > +cleanup: > + > + esxVI_ScsiLun_Free(&scsiLunList); Needs to free propertyNameList and hostSystem too. > + return result; > + > +} > diff --git a/src/esx/esx_vi_generator.input b/src/esx/esx_vi_generator.input > index c4a3e56..21f5b10 100644 > --- a/src/esx/esx_vi_generator.input > +++ b/src/esx/esx_vi_generator.input > @@ -128,6 +136,12 @@ enum VirtualMachinePowerState > end > > > +enum vStorageSupport > + vStorageUnknown > +end This enum is not used anywhere and can be removed. ACK. No major problems left. I fixed the minor things I mentioned, made make syntax-check pass and pushed the result. Thanks! -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list