[libvirt] [PATCH v2 00/12] esx: various improvements
- implement connectListAllStoragePools, so virConnectListAllStoragePools() works - implement connectListAllNetworks, so virConnectListAllNetworks() works - implement storagePoolListAllVolumes, so virStoragePoolListAllVolumes() works - set the proper filesystem type for vmfs-based datastores Pino Toscano (12): esx: split datastoreToStoragePoolPtr helper esx: split datastorePoolType helper esx: split targetToStoragePool helper esx: implement connectListAllStoragePools esx: split virtualswitchToNetwork helper esx: implement connectListAllNetworks storage: add vmfs filesystem type esx: set vmfs fs type for vmfs-based datastores esx: split datastorePathToStorageVol helper esx: split scsilunToStorageVol helper esx: implement storagePoolListAllVolumes docs: document implemented APIs in esx docs/news.xml | 11 + docs/schemas/storagepool.rng | 1 + docs/schemas/storagevol.rng | 1 + docs/storage.html.in | 3 + src/conf/storage_conf.c | 1 + src/conf/storage_conf.h | 1 + src/esx/esx_network_driver.c | 100 +++++- src/esx/esx_storage_backend_iscsi.c | 234 +++++++++++--- src/esx/esx_storage_backend_vmfs.c | 306 +++++++++++++++--- src/esx/esx_storage_driver.c | 87 +++++ .../storagepoolcapsschemadata/poolcaps-fs.xml | 1 + .../poolcaps-full.xml | 1 + 12 files changed, 655 insertions(+), 92 deletions(-) -- 2.21.0
Move the creation of a virStoragePtr object from the esxVI_ObjectContent object of a datastore out of esxStoragePoolLookupByName in an own helper. This way it can be used also in other functions. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_vmfs.c | 45 ++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 5f25f80072..78fe2b598d 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -195,26 +195,16 @@ esxConnectListStoragePools(virConnectPtr conn, char **const names, static virStoragePoolPtr -esxStoragePoolLookupByName(virConnectPtr conn, - const char *name) +datastoreToStoragePoolPtr(virConnectPtr conn, + const char *name, + esxVI_ObjectContent *datastore) { esxPrivate *priv = conn->privateData; - esxVI_ObjectContent *datastore = NULL; esxVI_DatastoreHostMount *hostMount = NULL; /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; virStoragePoolPtr pool = NULL; - if (esxVI_LookupDatastoreByName(priv->primary, name, NULL, &datastore, - esxVI_Occurrence_OptionalItem) < 0) { - goto cleanup; - } - - if (!datastore) { - /* Not found, let the base storage driver handle error reporting */ - goto cleanup; - } - /* * Datastores don't have a UUID, but we can use the 'host.mountInfo.path' * property as source for a UUID. The mount path is unique per host and @@ -239,7 +229,6 @@ esxStoragePoolLookupByName(virConnectPtr conn, pool = virGetStoragePool(conn, name, md5, &esxStorageBackendVMFS, NULL); cleanup: - esxVI_ObjectContent_Free(&datastore); esxVI_DatastoreHostMount_Free(&hostMount); return pool; @@ -247,6 +236,34 @@ esxStoragePoolLookupByName(virConnectPtr conn, +static virStoragePoolPtr +esxStoragePoolLookupByName(virConnectPtr conn, + const char *name) +{ + esxPrivate *priv = conn->privateData; + esxVI_ObjectContent *datastore = NULL; + virStoragePoolPtr pool = NULL; + + if (esxVI_LookupDatastoreByName(priv->primary, name, NULL, &datastore, + esxVI_Occurrence_OptionalItem) < 0) { + goto cleanup; + } + + if (!datastore) { + /* Not found, let the base storage driver handle error reporting */ + goto cleanup; + } + + pool = datastoreToStoragePoolPtr(conn, name, datastore); + + cleanup: + esxVI_ObjectContent_Free(&datastore); + + return pool; +} + + + static virStoragePoolPtr esxStoragePoolLookupByUUID(virConnectPtr conn, const unsigned char *uuid) -- 2.21.0
On Fri, Nov 15, 2019 at 01:40:40PM +0100, Pino Toscano wrote:
Move the creation of a virStoragePtr object from the esxVI_ObjectContent object of a datastore out of esxStoragePoolLookupByName in an own helper. This way it can be used also in other functions.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_vmfs.c | 45 ++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 14 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Move the detection of the type of a vmfs pool out of esxLookupVMFSStoragePoolType in an own helper. This way it can be used also in other functions. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_vmfs.c | 49 ++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 78fe2b598d..b890825a40 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -54,26 +54,12 @@ verify(VIR_CRYPTO_HASH_SIZE_MD5 == VIR_UUID_BUFLEN); static int -esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const char *poolName, - int *poolType) +datastorePoolType(esxVI_ObjectContent *datastore, int *poolType) { int result = -1; - esxVI_String *propertyNameList = NULL; - esxVI_ObjectContent *datastore = NULL; esxVI_DynamicProperty *dynamicProperty = NULL; esxVI_DatastoreInfo *datastoreInfo = NULL; - if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || - esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList, &datastore, - esxVI_Occurrence_OptionalItem) < 0) { - goto cleanup; - } - - if (!datastore) { - /* Not found, let the base storage driver handle error reporting */ - goto cleanup; - } - for (dynamicProperty = datastore->propSet; dynamicProperty; dynamicProperty = dynamicProperty->_next) { if (STREQ(dynamicProperty->name, "info")) { @@ -100,10 +86,41 @@ esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const char *poolName, result = 0; + cleanup: + esxVI_DatastoreInfo_Free(&datastoreInfo); + + return result; +} + + + +static int +esxLookupVMFSStoragePoolType(esxVI_Context *ctx, const char *poolName, + int *poolType) +{ + int result = -1; + esxVI_String *propertyNameList = NULL; + esxVI_ObjectContent *datastore = NULL; + + if (esxVI_String_AppendValueToList(&propertyNameList, "info") < 0 || + esxVI_LookupDatastoreByName(ctx, poolName, propertyNameList, &datastore, + esxVI_Occurrence_OptionalItem) < 0) { + goto cleanup; + } + + if (!datastore) { + /* Not found, let the base storage driver handle error reporting */ + goto cleanup; + } + + if (datastorePoolType(datastore, poolType) < 0) + goto cleanup; + + result = 0; + cleanup: esxVI_String_Free(&propertyNameList); esxVI_ObjectContent_Free(&datastore); - esxVI_DatastoreInfo_Free(&datastoreInfo); return result; } -- 2.21.0
On Fri, Nov 15, 2019 at 01:40:41PM +0100, Pino Toscano wrote:
Move the detection of the type of a vmfs pool out of esxLookupVMFSStoragePoolType in an own helper. This way it can be used also in other functions.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_vmfs.c | 49 ++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Move the creation of a virStoragePtr object from the esxVI_HostInternetScsiHbaStaticTarget object of a target out of esxStoragePoolLookupByName in an own helper. This way it can be used also in other functions. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_iscsi.c | 32 +++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 61354a6938..72ab0d3cb0 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -147,14 +147,32 @@ esxConnectListStoragePools(virConnectPtr conn, char **const names, +static virStoragePoolPtr +targetToStoragePool(virConnectPtr conn, + const char *name, + esxVI_HostInternetScsiHbaStaticTarget *target) +{ + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; + + /* + * HostInternetScsiHbaStaticTarget does not provide a uuid field, + * but iScsiName (or widely known as IQN) is unique across the multiple + * hosts, using it to compute key + */ + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, target->iScsiName, md5) < 0) + return NULL; + + return virGetStoragePool(conn, name, md5, &esxStorageBackendISCSI, NULL); +} + + static virStoragePoolPtr esxStoragePoolLookupByName(virConnectPtr conn, const char *name) { esxPrivate *priv = conn->privateData; esxVI_HostInternetScsiHbaStaticTarget *target = NULL; - /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; virStoragePoolPtr pool = NULL; /* @@ -172,15 +190,7 @@ esxStoragePoolLookupByName(virConnectPtr conn, goto cleanup; } - /* - * HostInternetScsiHbaStaticTarget does not provide a uuid field, - * but iScsiName (or widely known as IQN) is unique across the multiple - * hosts, using it to compute key - */ - if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, target->iScsiName, md5) < 0) - goto cleanup; - - pool = virGetStoragePool(conn, name, md5, &esxStorageBackendISCSI, NULL); + pool = targetToStoragePool(conn, name, target); cleanup: esxVI_HostInternetScsiHbaStaticTarget_Free(&target); -- 2.21.0
On Fri, Nov 15, 2019 at 01:40:42PM +0100, Pino Toscano wrote:
Move the creation of a virStoragePtr object from the esxVI_HostInternetScsiHbaStaticTarget object of a target out of esxStoragePoolLookupByName in an own helper. This way it can be used also in other functions.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_iscsi.c | 32 +++++++++++++++++++---------- 1 file changed, 21 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Implement the .connectListAllStoragePools storage API in the esx storage driver, and in all its subdrivers. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_iscsi.c | 72 +++++++++++++++++++++ src/esx/esx_storage_backend_vmfs.c | 98 +++++++++++++++++++++++++++++ src/esx/esx_storage_driver.c | 68 ++++++++++++++++++++ 3 files changed, 238 insertions(+) diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 72ab0d3cb0..4f5d8e5e24 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -779,6 +779,77 @@ esxStorageVolGetPath(virStorageVolPtr volume) +#define MATCH(FLAG) (flags & (FLAG)) +static int +esxConnectListAllStoragePools(virConnectPtr conn, + virStoragePoolPtr **pools, + unsigned int flags) +{ + bool success = false; + size_t count = 0; + esxPrivate *priv = conn->privateData; + esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL; + esxVI_HostInternetScsiHbaStaticTarget *target; + size_t i; + + /* this driver provides only iSCSI pools */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) && + !(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI))) + return 0; + + if (esxVI_LookupHostInternetScsiHba(priv->primary, + &hostInternetScsiHba) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to obtain iSCSI adapter")); + goto cleanup; + } + + /* FIXME: code looks for software iSCSI adapter only */ + if (!hostInternetScsiHba) { + /* iSCSI adapter may not be enabled for this host */ + return 0; + } + + /* + * ESX has two kind of targets: + * 1. staticIscsiTargets + * 2. dynamicIscsiTargets + * For each dynamic target if its reachable a static target is added. + * return iSCSI names for all static targets to avoid duplicate names. + */ + for (target = hostInternetScsiHba->configuredStaticTarget; + target; target = target->_next) { + virStoragePoolPtr pool; + + pool = targetToStoragePool(conn, target->iScsiName, target); + if (!pool) + goto cleanup; + + if (VIR_APPEND_ELEMENT(*pools, count, pool) < 0) + goto cleanup; + } + + success = true; + + cleanup: + if (! success) { + if (*pools) { + for (i = 0; i < count; ++i) + VIR_FREE((*pools)[i]); + VIR_FREE(*pools); + } + + count = -1; + } + + esxVI_HostInternetScsiHba_Free(&hostInternetScsiHba); + + return count; +} +#undef MATCH + + + virStorageDriver esxStorageBackendISCSI = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */ .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */ @@ -799,4 +870,5 @@ virStorageDriver esxStorageBackendISCSI = { .storageVolDelete = esxStorageVolDelete, /* 1.0.1 */ .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */ .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */ + .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */ }; diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index b890825a40..05b273aed7 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -1460,6 +1460,103 @@ esxStorageVolGetPath(virStorageVolPtr volume) +#define MATCH(FLAG) (flags & (FLAG)) +static int +esxConnectListAllStoragePools(virConnectPtr conn, + virStoragePoolPtr **pools, + unsigned int flags) +{ + bool success = false; + esxPrivate *priv = conn->privateData; + esxVI_String *propertyNameList = NULL; + esxVI_DynamicProperty *dynamicProperty = NULL; + esxVI_ObjectContent *datastoreList = NULL; + esxVI_ObjectContent *datastore = NULL; + size_t count = 0; + size_t i; + virStoragePoolPtr pool; + const bool checkPoolType = MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE); + + if (esxVI_String_AppendValueToList(&propertyNameList, + "summary.name") < 0) { + goto cleanup; + } + + if (checkPoolType && + esxVI_String_AppendValueToList(&propertyNameList, + "info") < 0) { + goto cleanup; + } + + if (esxVI_LookupDatastoreList(priv->primary, propertyNameList, + &datastoreList) < 0) { + goto cleanup; + } + + for (datastore = datastoreList; datastore; + datastore = datastore->_next) { + const char *name = NULL; + + for (dynamicProperty = datastore->propSet; dynamicProperty; + dynamicProperty = dynamicProperty->_next) { + if (STREQ(dynamicProperty->name, "summary.name")) { + if (esxVI_AnyType_ExpectType(dynamicProperty->val, + esxVI_Type_String) < 0) { + goto cleanup; + } + + name = dynamicProperty->val->string; + } + } + + if (!name) + goto cleanup; + + if (checkPoolType) { + int poolType; + + if (datastorePoolType(datastore, &poolType) < 0) + goto cleanup; + + if (!((MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_DIR) && + (poolType == VIR_STORAGE_POOL_DIR)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FS) && + (poolType == VIR_STORAGE_POOL_FS)) || + (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_NETFS) && + (poolType == VIR_STORAGE_POOL_NETFS)))) + continue; + } + + pool = datastoreToStoragePoolPtr(conn, name, datastore); + if (!pool) + goto cleanup; + + if (VIR_APPEND_ELEMENT(*pools, count, pool) < 0) + goto cleanup; + } + + success = true; + + cleanup: + if (! success) { + if (*pools) { + for (i = 0; i < count; ++i) + VIR_FREE((*pools)[i]); + VIR_FREE(*pools); + } + + count = -1; + } + + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastoreList); + + return count; +} +#undef MATCH + + + virStorageDriver esxStorageBackendVMFS = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 0.8.2 */ .connectListStoragePools = esxConnectListStoragePools, /* 0.8.2 */ @@ -1480,4 +1577,5 @@ virStorageDriver esxStorageBackendVMFS = { .storageVolGetInfo = esxStorageVolGetInfo, /* 0.8.4 */ .storageVolGetXMLDesc = esxStorageVolGetXMLDesc, /* 0.8.4 */ .storageVolGetPath = esxStorageVolGetPath, /* 0.8.4 */ + .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */ }; diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 8a34732b45..6d17ac28ea 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -517,6 +517,73 @@ esxStoragePoolIsPersistent(virStoragePoolPtr pool G_GNUC_UNUSED) +#define MATCH(FLAG) (flags & (FLAG)) +static int +esxConnectListAllStoragePools(virConnectPtr conn, + virStoragePoolPtr **pools, + unsigned int flags) +{ + bool success = false; + esxPrivate *priv = conn->privateData; + size_t count = 0; + size_t i, j; + int tmp; + + virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL, -1); + + /* + * ESX storage pools are always active, persistent, and + * autostarted, so return zero elements in case we are asked + * for pools different than that. + * + * Filtering by type will be done by each backend. + */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) && + !(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE))) + return 0; + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_PERSISTENT) && + !(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT))) + return 0; + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_AUTOSTART) && + !(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART))) + return 0; + + if (esxVI_EnsureSession(priv->primary) < 0) + return -1; + + for (i = 0; i < LAST_BACKEND; ++i) { + virStoragePoolPtr *new_pools = 0; + tmp = backends[i]->connectListAllStoragePools(conn, &new_pools, flags); + + if (tmp < 0) + goto cleanup; + + for (j = 0; j < tmp; ++j) { + if (VIR_APPEND_ELEMENT(*pools, count, new_pools[j]) < 0) + goto cleanup; + } + VIR_FREE(new_pools); + } + + success = true; + + cleanup: + if (! success) { + if (*pools) { + for (i = 0; i < count; ++i) + VIR_FREE((*pools)[i]); + VIR_FREE(*pools); + } + + count = -1; + } + + return count; +} +#undef MATCH + + + virStorageDriver esxStorageDriver = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 0.8.2 */ .connectListStoragePools = esxConnectListStoragePools, /* 0.8.2 */ @@ -544,4 +611,5 @@ virStorageDriver esxStorageDriver = { .storageVolGetPath = esxStorageVolGetPath, /* 0.8.4 */ .storagePoolIsActive = esxStoragePoolIsActive, /* 0.8.2 */ .storagePoolIsPersistent = esxStoragePoolIsPersistent, /* 0.8.2 */ + .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */ }; -- 2.21.0
On Fri, Nov 15, 2019 at 01:40:43PM +0100, Pino Toscano wrote:
Implement the .connectListAllStoragePools storage API in the esx storage driver, and in all its subdrivers.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_iscsi.c | 72 +++++++++++++++++++++ src/esx/esx_storage_backend_vmfs.c | 98 +++++++++++++++++++++++++++++ src/esx/esx_storage_driver.c | 68 ++++++++++++++++++++ 3 files changed, 238 insertions(+)
diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 72ab0d3cb0..4f5d8e5e24 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -779,6 +779,77 @@ esxStorageVolGetPath(virStorageVolPtr volume)
+#define MATCH(FLAG) (flags & (FLAG)) +static int +esxConnectListAllStoragePools(virConnectPtr conn, + virStoragePoolPtr **pools, + unsigned int flags) +{ + bool success = false; + size_t count = 0;
size_t is unsigned... [0]
+ esxPrivate *priv = conn->privateData; + esxVI_HostInternetScsiHba *hostInternetScsiHba = NULL; + esxVI_HostInternetScsiHbaStaticTarget *target; + size_t i; + + /* this driver provides only iSCSI pools */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_POOL_TYPE) && + !(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ISCSI))) + return 0; + + if (esxVI_LookupHostInternetScsiHba(priv->primary, + &hostInternetScsiHba) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to obtain iSCSI adapter")); + goto cleanup; + } + + /* FIXME: code looks for software iSCSI adapter only */ + if (!hostInternetScsiHba) { + /* iSCSI adapter may not be enabled for this host */ + return 0; + } + + /* + * ESX has two kind of targets: + * 1. staticIscsiTargets + * 2. dynamicIscsiTargets + * For each dynamic target if its reachable a static target is added.
it's
+ * return iSCSI names for all static targets to avoid duplicate names. + */ + for (target = hostInternetScsiHba->configuredStaticTarget; + target; target = target->_next) { + virStoragePoolPtr pool; + + pool = targetToStoragePool(conn, target->iScsiName, target); + if (!pool) + goto cleanup; + + if (VIR_APPEND_ELEMENT(*pools, count, pool) < 0) + goto cleanup; + } + + success = true; + + cleanup: + if (! success) { + if (*pools) { + for (i = 0; i < count; ++i) + VIR_FREE((*pools)[i]); + VIR_FREE(*pools); + } + + count = -1;
[0] so assigning negative value feels wrong. using int ret = -1; ... ret = count; cleanup: if (ret < 0) { } gets rid of that assignment.
+ } + + esxVI_HostInternetScsiHba_Free(&hostInternetScsiHba); + + return count; +} +#undef MATCH + + + virStorageDriver esxStorageBackendISCSI = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */ .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */ @@ -799,4 +870,5 @@ virStorageDriver esxStorageBackendISCSI = { .storageVolDelete = esxStorageVolDelete, /* 1.0.1 */ .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */ .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */ + .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */
6.0.0
}; diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index b890825a40..05b273aed7 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c
[...]
+ + success = true; + + cleanup: + if (! success) { + if (*pools) { + for (i = 0; i < count; ++i) + VIR_FREE((*pools)[i]); + VIR_FREE(*pools); + } + + count = -1;
same here
+ } + + esxVI_String_Free(&propertyNameList); + esxVI_ObjectContent_Free(&datastoreList); + + return count; +} +#undef MATCH + + + virStorageDriver esxStorageBackendVMFS = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 0.8.2 */ .connectListStoragePools = esxConnectListStoragePools, /* 0.8.2 */ @@ -1480,4 +1577,5 @@ virStorageDriver esxStorageBackendVMFS = { .storageVolGetInfo = esxStorageVolGetInfo, /* 0.8.4 */ .storageVolGetXMLDesc = esxStorageVolGetXMLDesc, /* 0.8.4 */ .storageVolGetPath = esxStorageVolGetPath, /* 0.8.4 */ + .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */
6.0.0
}; diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 8a34732b45..6d17ac28ea 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -517,6 +517,73 @@ esxStoragePoolIsPersistent(virStoragePoolPtr pool G_GNUC_UNUSED)
+#define MATCH(FLAG) (flags & (FLAG)) +static int +esxConnectListAllStoragePools(virConnectPtr conn, + virStoragePoolPtr **pools, + unsigned int flags) +{ + bool success = false; + esxPrivate *priv = conn->privateData; + size_t count = 0; + size_t i, j; + int tmp; + + virCheckFlags(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ALL, -1); + + /* + * ESX storage pools are always active, persistent, and + * autostarted, so return zero elements in case we are asked + * for pools different than that. + * + * Filtering by type will be done by each backend. + */ + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_ACTIVE) && + !(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE))) + return 0; + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_PERSISTENT) && + !(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_PERSISTENT))) + return 0; + if (MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_FILTERS_AUTOSTART) && + !(MATCH(VIR_CONNECT_LIST_STORAGE_POOLS_AUTOSTART))) + return 0; + + if (esxVI_EnsureSession(priv->primary) < 0) + return -1; + + for (i = 0; i < LAST_BACKEND; ++i) {
tmp is only used here so it can be declared here
+ virStoragePoolPtr *new_pools = 0; + tmp = backends[i]->connectListAllStoragePools(conn, &new_pools, flags); + + if (tmp < 0) + goto cleanup; + + for (j = 0; j < tmp; ++j) { + if (VIR_APPEND_ELEMENT(*pools, count, new_pools[j]) < 0) + goto cleanup; + } + VIR_FREE(new_pools); + } + + success = true; + + cleanup: + if (! success) { + if (*pools) { + for (i = 0; i < count; ++i) + VIR_FREE((*pools)[i]); + VIR_FREE(*pools); + } + + count = -1;
Same comment about int ret
+ } + + return count; +} +#undef MATCH + + + virStorageDriver esxStorageDriver = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 0.8.2 */ .connectListStoragePools = esxConnectListStoragePools, /* 0.8.2 */ @@ -544,4 +611,5 @@ virStorageDriver esxStorageDriver = { .storageVolGetPath = esxStorageVolGetPath, /* 0.8.4 */ .storagePoolIsActive = esxStoragePoolIsActive, /* 0.8.2 */ .storagePoolIsPersistent = esxStoragePoolIsPersistent, /* 0.8.2 */ + .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */
6.0.0 With that fixed: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Move the creation of a virNetworkPtr object from the esxVI_HostVirtualSwitch object of a virtual switch out of esxNetworkLookupByName in an own helper. This way it can be used also in other functions. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_network_driver.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index b5dcfe0a80..4f359c61e2 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -167,20 +167,11 @@ esxNetworkLookupByUUID(virConnectPtr conn, const unsigned char *uuid) static virNetworkPtr -esxNetworkLookupByName(virConnectPtr conn, const char *name) +virtualswitchToNetwork(virConnectPtr conn, + esxVI_HostVirtualSwitch *hostVirtualSwitch) { - virNetworkPtr network = NULL; - esxPrivate *priv = conn->privateData; - esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ - if (esxVI_EnsureSession(priv->primary) < 0 || - esxVI_LookupHostVirtualSwitchByName(priv->primary, name, - &hostVirtualSwitch, - esxVI_Occurrence_RequiredItem) < 0) { - return NULL; - } - /* * HostVirtualSwitch doesn't have a UUID, but we can use the key property * as source for a UUID. The key is unique per host and cannot change @@ -192,7 +183,26 @@ esxNetworkLookupByName(virConnectPtr conn, const char *name) if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, hostVirtualSwitch->key, md5) < 0) return NULL; - network = virGetNetwork(conn, hostVirtualSwitch->name, md5); + return virGetNetwork(conn, hostVirtualSwitch->name, md5); +} + + + +static virNetworkPtr +esxNetworkLookupByName(virConnectPtr conn, const char *name) +{ + virNetworkPtr network = NULL; + esxPrivate *priv = conn->privateData; + esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; + + if (esxVI_EnsureSession(priv->primary) < 0 || + esxVI_LookupHostVirtualSwitchByName(priv->primary, name, + &hostVirtualSwitch, + esxVI_Occurrence_RequiredItem) < 0) { + return NULL; + } + + network = virtualswitchToNetwork(conn, hostVirtualSwitch); esxVI_HostVirtualSwitch_Free(&hostVirtualSwitch); -- 2.21.0
On 11/15/19 7:40 AM, Pino Toscano wrote:
Move the creation of a virNetworkPtr object from the esxVI_HostVirtualSwitch object of a virtual switch out of esxNetworkLookupByName in an own helper. This way it can be used also in other functions.
Signed-off-by: Pino Toscano <ptoscano@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole
Implement the .connectListAllNetworks networks API in the esx network driver. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_network_driver.c | 66 ++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 4f359c61e2..79a7ecae5f 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -863,6 +863,71 @@ esxNetworkIsPersistent(virNetworkPtr network G_GNUC_UNUSED) +#define MATCH(FLAG) (flags & (FLAG)) +static int +esxConnectListAllNetworks(virConnectPtr conn, + virNetworkPtr **nets, + unsigned int flags) +{ + bool success = false; + esxPrivate *priv = conn->privateData; + esxVI_HostVirtualSwitch *hostVirtualSwitchList = NULL; + esxVI_HostVirtualSwitch *hostVirtualSwitch = NULL; + size_t count = 0; + size_t i; + + virCheckFlags(VIR_CONNECT_LIST_NETWORKS_FILTERS_ALL, -1); + + /* + * ESX networks are always active, persistent, and + * autostarted, so return zero elements in case we are asked + * for networks different than that. + */ + if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_ACTIVE) && + !(MATCH(VIR_CONNECT_LIST_NETWORKS_ACTIVE))) + return 0; + if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_PERSISTENT) && + !(MATCH(VIR_CONNECT_LIST_NETWORKS_PERSISTENT))) + return 0; + if (MATCH(VIR_CONNECT_LIST_NETWORKS_FILTERS_AUTOSTART) && + !(MATCH(VIR_CONNECT_LIST_NETWORKS_AUTOSTART))) + return 0; + + if (esxVI_EnsureSession(priv->primary) < 0 || + esxVI_LookupHostVirtualSwitchList(priv->primary, + &hostVirtualSwitchList) < 0) { + return -1; + } + + for (hostVirtualSwitch = hostVirtualSwitchList; hostVirtualSwitch; + hostVirtualSwitch = hostVirtualSwitch->_next) { + virNetworkPtr net = virtualswitchToNetwork(conn, hostVirtualSwitch); + + if (VIR_APPEND_ELEMENT(*nets, count, net) < 0) + goto cleanup; + } + + success = true; + + cleanup: + if (! success) { + if (*nets) { + for (i = 0; i < count; ++i) + VIR_FREE((*nets)[i]); + VIR_FREE(*nets); + } + + count = -1; + } + + esxVI_HostVirtualSwitch_Free(&hostVirtualSwitchList); + + return count; +} +#undef MATCH + + + virNetworkDriver esxNetworkDriver = { .connectNumOfNetworks = esxConnectNumOfNetworks, /* 0.10.0 */ .connectListNetworks = esxConnectListNetworks, /* 0.10.0 */ @@ -877,4 +942,5 @@ virNetworkDriver esxNetworkDriver = { .networkSetAutostart = esxNetworkSetAutostart, /* 0.10.0 */ .networkIsActive = esxNetworkIsActive, /* 0.10.0 */ .networkIsPersistent = esxNetworkIsPersistent, /* 0.10.0 */ + .connectListAllNetworks = esxConnectListAllNetworks, /* 5.10.0 */ }; -- 2.21.0
On 11/15/19 7:40 AM, Pino Toscano wrote:
Implement the .connectListAllNetworks networks API in the esx network driver.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_network_driver.c | 66 ++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+)
Needs the same treatment Jano pointed out in patch #4, size_t adjustment, and versions changes to 6.0.0. With that: Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole
It will be used to represent the type of a filesystem pool in ESXi. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- docs/schemas/storagepool.rng | 1 + docs/schemas/storagevol.rng | 1 + docs/storage.html.in | 3 +++ src/conf/storage_conf.c | 1 + src/conf/storage_conf.h | 1 + tests/storagepoolcapsschemadata/poolcaps-fs.xml | 1 + tests/storagepoolcapsschemadata/poolcaps-full.xml | 1 + 7 files changed, 9 insertions(+) diff --git a/docs/schemas/storagepool.rng b/docs/schemas/storagepool.rng index 976a02baeb..ff0d3c836c 100644 --- a/docs/schemas/storagepool.rng +++ b/docs/schemas/storagepool.rng @@ -451,6 +451,7 @@ <value>hfs+</value> <value>xfs</value> <value>ocfs2</value> + <value>vmfs</value> </choice> </attribute> </element> diff --git a/docs/schemas/storagevol.rng b/docs/schemas/storagevol.rng index 32aaa2784d..382cd121ad 100644 --- a/docs/schemas/storagevol.rng +++ b/docs/schemas/storagevol.rng @@ -194,6 +194,7 @@ <value>hfs+</value> <value>xfs</value> <value>ocfs2</value> + <value>vmfs</value> </choice> </define> diff --git a/docs/storage.html.in b/docs/storage.html.in index e0e4edec1e..72fa13944a 100644 --- a/docs/storage.html.in +++ b/docs/storage.html.in @@ -202,6 +202,9 @@ <li> <code>ocfs2</code> </li> + <li> + <code>vmfs</code> + </li> </ul> <h3>Valid volume format types</h3> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index fd640bfa2b..252d28cbfb 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -67,6 +67,7 @@ VIR_ENUM_IMPL(virStoragePoolFormatFileSystem, "auto", "ext2", "ext3", "ext4", "ufs", "iso9660", "udf", "gfs", "gfs2", "vfat", "hfs+", "xfs", "ocfs2", + "vmfs", ); VIR_ENUM_IMPL(virStoragePoolFormatFileSystemNet, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index d2600efff0..c0baeffc1c 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -362,6 +362,7 @@ typedef enum { VIR_STORAGE_POOL_FS_HFSPLUS, VIR_STORAGE_POOL_FS_XFS, VIR_STORAGE_POOL_FS_OCFS2, + VIR_STORAGE_POOL_FS_VMFS, VIR_STORAGE_POOL_FS_LAST, } virStoragePoolFormatFileSystem; VIR_ENUM_DECL(virStoragePoolFormatFileSystem); diff --git a/tests/storagepoolcapsschemadata/poolcaps-fs.xml b/tests/storagepoolcapsschemadata/poolcaps-fs.xml index 182fa398f5..eee75af746 100644 --- a/tests/storagepoolcapsschemadata/poolcaps-fs.xml +++ b/tests/storagepoolcapsschemadata/poolcaps-fs.xml @@ -40,6 +40,7 @@ <value>hfs+</value> <value>xfs</value> <value>ocfs2</value> + <value>vmfs</value> </enum> </poolOptions> <volOptions> diff --git a/tests/storagepoolcapsschemadata/poolcaps-full.xml b/tests/storagepoolcapsschemadata/poolcaps-full.xml index 980c6d210e..805950a937 100644 --- a/tests/storagepoolcapsschemadata/poolcaps-full.xml +++ b/tests/storagepoolcapsschemadata/poolcaps-full.xml @@ -40,6 +40,7 @@ <value>hfs+</value> <value>xfs</value> <value>ocfs2</value> + <value>vmfs</value> </enum> </poolOptions> <volOptions> -- 2.21.0
On 11/15/19 7:40 AM, Pino Toscano wrote:
It will be used to represent the type of a filesystem pool in ESXi.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- docs/schemas/storagepool.rng | 1 + docs/schemas/storagevol.rng | 1 + docs/storage.html.in | 3 +++ src/conf/storage_conf.c | 1 + src/conf/storage_conf.h | 1 + tests/storagepoolcapsschemadata/poolcaps-fs.xml | 1 + tests/storagepoolcapsschemadata/poolcaps-full.xml | 1 + 7 files changed, 9 insertions(+)
Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole
This way they are correctly represented: <source> <format type='vmfs'/> </source> ... instead of 'auto'. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_vmfs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 05b273aed7..1270c21e00 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -531,6 +531,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) } } else if (esxVI_VmfsDatastoreInfo_DynamicCast(info)) { def.type = VIR_STORAGE_POOL_FS; + def.source.format = VIR_STORAGE_POOL_FS_VMFS; /* * FIXME: I'm not sure how to represent the source and target of a * VMFS based datastore in libvirt terms -- 2.21.0
On 11/15/19 7:40 AM, Pino Toscano wrote:
This way they are correctly represented: <source> <format type='vmfs'/> </source> ... instead of 'auto'.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_vmfs.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 05b273aed7..1270c21e00 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -531,6 +531,7 @@ esxStoragePoolGetXMLDesc(virStoragePoolPtr pool, unsigned int flags) } } else if (esxVI_VmfsDatastoreInfo_DynamicCast(info)) { def.type = VIR_STORAGE_POOL_FS; + def.source.format = VIR_STORAGE_POOL_FS_VMFS; /* * FIXME: I'm not sure how to represent the source and target of a * VMFS based datastore in libvirt terms
Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole
Move the creation of a virStorageVolPtr object by lookup out of esxStorageVolLookupByPath in an own helper. This way it can be used also in other functions. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_vmfs.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 1270c21e00..ab47a4c272 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -695,32 +695,40 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, +static virStorageVolPtr +datastorePathToStorageVol(virConnectPtr conn, const char *pool, + const char *path) +{ + esxPrivate *priv = conn->privateData; + g_autofree char *key = NULL; + + if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path, + &key) < 0) { + return NULL; + } + + return virGetStorageVol(conn, pool, path, key, + &esxStorageBackendVMFS, NULL); +} + + static virStorageVolPtr esxStorageVolLookupByPath(virConnectPtr conn, const char *path) { virStorageVolPtr volume = NULL; - esxPrivate *priv = conn->privateData; char *datastoreName = NULL; char *directoryAndFileName = NULL; - char *key = NULL; if (esxUtil_ParseDatastorePath(path, &datastoreName, NULL, &directoryAndFileName) < 0) { goto cleanup; } - if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path, - &key) < 0) { - goto cleanup; - } - - volume = virGetStorageVol(conn, datastoreName, directoryAndFileName, key, - &esxStorageBackendVMFS, NULL); + volume = datastorePathToStorageVol(conn, datastoreName, directoryAndFileName); cleanup: VIR_FREE(datastoreName); VIR_FREE(directoryAndFileName); - VIR_FREE(key); return volume; } -- 2.21.0
On 11/15/19 7:40 AM, Pino Toscano wrote:
Move the creation of a virStorageVolPtr object by lookup out of esxStorageVolLookupByPath in an own helper. This way it can be used also in other functions.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_vmfs.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index 1270c21e00..ab47a4c272 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -695,32 +695,40 @@ esxStorageVolLookupByName(virStoragePoolPtr pool,
+static virStorageVolPtr +datastorePathToStorageVol(virConnectPtr conn, const char *pool, + const char *path) +{ + esxPrivate *priv = conn->privateData; + g_autofree char *key = NULL; + + if (esxVI_LookupStorageVolumeKeyByDatastorePath(priv->primary, path, + &key) < 0) { + return NULL; + } + + return virGetStorageVol(conn, pool, path, key, + &esxStorageBackendVMFS, NULL); +} + +
Add an extra newline here, since this file uses triple line spacing Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole
Move the creation of a virStorageVolPtr object from the esxVI_ScsiLun object of a SCSI lun out of esxStorageVolLookupByName and esxStorageVolLookupByPath in an own helper. This way it can be used also in other functions. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_iscsi.c | 60 +++++++++++++++-------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 4f5d8e5e24..34760a837e 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -440,6 +440,35 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names, +static virStorageVolPtr +scsilunToStorageVol(virConnectPtr conn, esxVI_ScsiLun *scsiLun, + const char *pool) +{ + /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ + unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; + char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; + + /* + * ScsiLun provides a UUID field that is unique across + * multiple servers. But this field length is ~55 characters + * compute MD5 hash to transform it to an acceptable + * libvirt format + */ + if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0) + return NULL; + virUUIDFormat(md5, uuid_string); + + /* + * ScsiLun provides displayName and canonicalName but both are + * optional and its observed that they can be NULL, using + * deviceName to create volume. + */ + return virGetStorageVol(conn, pool, scsiLun->deviceName, uuid_string, + &esxStorageBackendISCSI, NULL); +} + + + static virStorageVolPtr esxStorageVolLookupByName(virStoragePoolPtr pool, const char *name) @@ -448,9 +477,6 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, esxPrivate *priv = pool->conn->privateData; esxVI_ScsiLun *scsiLunList = NULL; esxVI_ScsiLun *scsiLun; - /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; - char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; if (esxVI_LookupScsiLunList(priv->primary, &scsiLunList) < 0) goto cleanup; @@ -458,23 +484,7 @@ esxStorageVolLookupByName(virStoragePoolPtr pool, for (scsiLun = scsiLunList; scsiLun; scsiLun = scsiLun->_next) { if (STREQ(scsiLun->deviceName, name)) { - /* - * ScsiLun provides a UUID field that is unique across - * multiple servers. But this field length is ~55 characters - * compute MD5 hash to transform it to an acceptable - * libvirt format - */ - if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0) - goto cleanup; - virUUIDFormat(md5, uuid_string); - - /* - * ScsiLun provides displayName and canonicalName but both are - * optional and its observed that they can be NULL, using - * deviceName to create volume. - */ - volume = virGetStorageVol(pool->conn, pool->name, name, uuid_string, - &esxStorageBackendISCSI, NULL); + volume = scsilunToStorageVol(pool->conn, scsiLun, pool->name); break; } } @@ -496,9 +506,6 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) esxVI_ScsiLun *scsiLun; esxVI_HostScsiDisk *hostScsiDisk = NULL; char *poolName = NULL; - /* VIR_CRYPTO_HASH_SIZE_MD5 = VIR_UUID_BUFLEN = 16 */ - unsigned char md5[VIR_CRYPTO_HASH_SIZE_MD5]; - char uuid_string[VIR_UUID_STRING_BUFLEN] = ""; if (esxVI_LookupScsiLunList(priv->primary, &scsiLunList) < 0) goto cleanup; @@ -516,12 +523,7 @@ esxStorageVolLookupByPath(virConnectPtr conn, const char *path) goto cleanup; } - if (virCryptoHashBuf(VIR_CRYPTO_HASH_MD5, scsiLun->uuid, md5) < 0) - goto cleanup; - virUUIDFormat(md5, uuid_string); - - volume = virGetStorageVol(conn, poolName, path, uuid_string, - &esxStorageBackendISCSI, NULL); + volume = scsilunToStorageVol(conn, scsiLun, poolName); break; } } -- 2.21.0
On 11/15/19 7:40 AM, Pino Toscano wrote:
Move the creation of a virStorageVolPtr object from the esxVI_ScsiLun object of a SCSI lun out of esxStorageVolLookupByName and esxStorageVolLookupByPath in an own helper. This way it can be used also in other functions.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_iscsi.c | 60 +++++++++++++++-------------- 1 file changed, 31 insertions(+), 29 deletions(-)
diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 4f5d8e5e24..34760a837e 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -440,6 +440,35 @@ esxStoragePoolListVolumes(virStoragePoolPtr pool, char **const names,
+static virStorageVolPtr +scsilunToStorageVol(virConnectPtr conn, esxVI_ScsiLun *scsiLun, + const char *pool)
I think this should be named 'scsiLunXXX' to be consistent with capitalization used elsewhere Reviewed-by: Cole Robinson <crobinso@redhat.com> - Cole
Implement the .storagePoolListAllVolumes storage API in the esx storage driver, and in all its subdrivers. Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_iscsi.c | 70 ++++++++++++++++++++++++ src/esx/esx_storage_backend_vmfs.c | 85 +++++++++++++++++++++++++++++ src/esx/esx_storage_driver.c | 19 +++++++ 3 files changed, 174 insertions(+) diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 34760a837e..2f189a92cf 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -852,6 +852,75 @@ esxConnectListAllStoragePools(virConnectPtr conn, +static int +esxStoragePoolListAllVolumes(virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags) +{ + bool success = false; + size_t count = 0; + esxPrivate *priv = pool->conn->privateData; + esxVI_HostScsiTopologyLun *hostScsiTopologyLunList = NULL; + esxVI_HostScsiTopologyLun *hostScsiTopologyLun; + esxVI_ScsiLun *scsiLunList = NULL; + esxVI_ScsiLun *scsiLun = NULL; + size_t i; + + virCheckFlags(0, -1); + + if (esxVI_LookupHostScsiTopologyLunListByTargetName + (priv->primary, pool->name, &hostScsiTopologyLunList) < 0) { + goto cleanup; + } + + if (!hostScsiTopologyLunList) { + /* iSCSI adapter may not be enabled on ESX host */ + return 0; + } + + if (esxVI_LookupScsiLunList(priv->primary, &scsiLunList) < 0) + goto cleanup; + + for (scsiLun = scsiLunList; scsiLun; + scsiLun = scsiLun->_next) { + for (hostScsiTopologyLun = hostScsiTopologyLunList; + hostScsiTopologyLun; + hostScsiTopologyLun = hostScsiTopologyLun->_next) { + if (STREQ(hostScsiTopologyLun->scsiLun, scsiLun->key)) { + virStorageVolPtr volume; + + volume = scsilunToStorageVol(pool->conn, scsiLun, pool->name); + if (!volume) + goto cleanup; + + if (VIR_APPEND_ELEMENT(*vols, count, volume) < 0) + goto cleanup; + } + + } + } + + success = true; + + cleanup: + if (! success) { + if (*vols) { + for (i = 0; i < count; ++i) + VIR_FREE((*vols)[i]); + VIR_FREE(*vols); + } + + count = -1; + } + + esxVI_HostScsiTopologyLun_Free(&hostScsiTopologyLunList); + esxVI_ScsiLun_Free(&scsiLunList); + + return count; +} + + + virStorageDriver esxStorageBackendISCSI = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */ .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */ @@ -873,4 +942,5 @@ virStorageDriver esxStorageBackendISCSI = { .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */ .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */ .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */ + .storagePoolListAllVolumes = esxStoragePoolListAllVolumes, /* 5.10.0 */ }; diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index ab47a4c272..27f8b17e06 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -1566,6 +1566,90 @@ esxConnectListAllStoragePools(virConnectPtr conn, +static int +esxStoragePoolListAllVolumes(virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags) +{ + bool success = false; + esxPrivate *priv = pool->conn->privateData; + esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL; + esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; + esxVI_FileInfo *fileInfo = NULL; + char *directoryAndFileName = NULL; + size_t length; + size_t count = 0; + size_t i; + + virCheckFlags(0, -1); + + if (esxVI_LookupDatastoreContentByDatastoreName(priv->primary, pool->name, + &searchResultsList) < 0) { + goto cleanup; + } + + /* Interpret search result */ + for (searchResults = searchResultsList; searchResults; + searchResults = searchResults->_next) { + VIR_FREE(directoryAndFileName); + + if (esxUtil_ParseDatastorePath(searchResults->folderPath, NULL, NULL, + &directoryAndFileName) < 0) { + goto cleanup; + } + + /* Strip trailing separators */ + length = strlen(directoryAndFileName); + + while (length > 0 && directoryAndFileName[length - 1] == '/') { + directoryAndFileName[length - 1] = '\0'; + --length; + } + + /* Build volume names */ + for (fileInfo = searchResults->file; fileInfo; + fileInfo = fileInfo->_next) { + g_autofree char *datastorePath = NULL; + virStorageVolPtr volume; + + if (length < 1) { + datastorePath = g_strdup(fileInfo->path); + } else { + datastorePath = g_strdup_printf("%s/%s", + directoryAndFileName, + fileInfo->path); + } + + volume = datastorePathToStorageVol(pool->conn, pool->name, datastorePath); + if (!volume) + goto cleanup; + + if (VIR_APPEND_ELEMENT(*vols, count, volume) < 0) + goto cleanup; + } + } + + success = true; + + cleanup: + if (! success) { + if (*vols) { + for (i = 0; i < count; ++i) + VIR_FREE((*vols)[i]); + VIR_FREE(*vols); + } + + count = -1; + } + + esxVI_HostDatastoreBrowserSearchResults_Free(&searchResultsList); + VIR_FREE(directoryAndFileName); + + return count; +} + + + virStorageDriver esxStorageBackendVMFS = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 0.8.2 */ .connectListStoragePools = esxConnectListStoragePools, /* 0.8.2 */ @@ -1587,4 +1671,5 @@ virStorageDriver esxStorageBackendVMFS = { .storageVolGetXMLDesc = esxStorageVolGetXMLDesc, /* 0.8.4 */ .storageVolGetPath = esxStorageVolGetPath, /* 0.8.4 */ .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */ + .storagePoolListAllVolumes = esxStoragePoolListAllVolumes, /* 5.10.0 */ }; diff --git a/src/esx/esx_storage_driver.c b/src/esx/esx_storage_driver.c index 6d17ac28ea..ba72a0e5b5 100644 --- a/src/esx/esx_storage_driver.c +++ b/src/esx/esx_storage_driver.c @@ -584,6 +584,24 @@ esxConnectListAllStoragePools(virConnectPtr conn, +static int +esxStoragePoolListAllVolumes(virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags) +{ + esxPrivate *priv = pool->conn->privateData; + virStorageDriverPtr backend = pool->privateData; + + virCheckNonNullArgReturn(pool->privateData, -1); + + if (esxVI_EnsureSession(priv->primary) < 0) + return -1; + + return backend->storagePoolListAllVolumes(pool, vols, flags); +} + + + virStorageDriver esxStorageDriver = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 0.8.2 */ .connectListStoragePools = esxConnectListStoragePools, /* 0.8.2 */ @@ -612,4 +630,5 @@ virStorageDriver esxStorageDriver = { .storagePoolIsActive = esxStoragePoolIsActive, /* 0.8.2 */ .storagePoolIsPersistent = esxStoragePoolIsPersistent, /* 0.8.2 */ .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */ + .storagePoolListAllVolumes = esxStoragePoolListAllVolumes, /* 5.10.0 */ }; -- 2.21.0
On 11/15/19 7:40 AM, Pino Toscano wrote:
Implement the .storagePoolListAllVolumes storage API in the esx storage driver, and in all its subdrivers.
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- src/esx/esx_storage_backend_iscsi.c | 70 ++++++++++++++++++++++++ src/esx/esx_storage_backend_vmfs.c | 85 +++++++++++++++++++++++++++++ src/esx/esx_storage_driver.c | 19 +++++++ 3 files changed, 174 insertions(+)
diff --git a/src/esx/esx_storage_backend_iscsi.c b/src/esx/esx_storage_backend_iscsi.c index 34760a837e..2f189a92cf 100644 --- a/src/esx/esx_storage_backend_iscsi.c +++ b/src/esx/esx_storage_backend_iscsi.c @@ -852,6 +852,75 @@ esxConnectListAllStoragePools(virConnectPtr conn,
+static int +esxStoragePoolListAllVolumes(virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags) +{ + bool success = false; + size_t count = 0; + esxPrivate *priv = pool->conn->privateData; + esxVI_HostScsiTopologyLun *hostScsiTopologyLunList = NULL; + esxVI_HostScsiTopologyLun *hostScsiTopologyLun; + esxVI_ScsiLun *scsiLunList = NULL; + esxVI_ScsiLun *scsiLun = NULL; + size_t i; + + virCheckFlags(0, -1); + + if (esxVI_LookupHostScsiTopologyLunListByTargetName + (priv->primary, pool->name, &hostScsiTopologyLunList) < 0) { + goto cleanup; + } + + if (!hostScsiTopologyLunList) { + /* iSCSI adapter may not be enabled on ESX host */ + return 0; + } + + if (esxVI_LookupScsiLunList(priv->primary, &scsiLunList) < 0) + goto cleanup; + + for (scsiLun = scsiLunList; scsiLun; + scsiLun = scsiLun->_next) { + for (hostScsiTopologyLun = hostScsiTopologyLunList; + hostScsiTopologyLun; + hostScsiTopologyLun = hostScsiTopologyLun->_next) { + if (STREQ(hostScsiTopologyLun->scsiLun, scsiLun->key)) { + virStorageVolPtr volume; + + volume = scsilunToStorageVol(pool->conn, scsiLun, pool->name); + if (!volume) + goto cleanup; + + if (VIR_APPEND_ELEMENT(*vols, count, volume) < 0) + goto cleanup; + } + + } + } + + success = true; + + cleanup: + if (! success) { + if (*vols) { + for (i = 0; i < count; ++i) + VIR_FREE((*vols)[i]); + VIR_FREE(*vols); + } + + count = -1; + } + + esxVI_HostScsiTopologyLun_Free(&hostScsiTopologyLunList); + esxVI_ScsiLun_Free(&scsiLunList); + + return count; +} + + + virStorageDriver esxStorageBackendISCSI = { .connectNumOfStoragePools = esxConnectNumOfStoragePools, /* 1.0.1 */ .connectListStoragePools = esxConnectListStoragePools, /* 1.0.1 */ @@ -873,4 +942,5 @@ virStorageDriver esxStorageBackendISCSI = { .storageVolWipe = esxStorageVolWipe, /* 1.0.1 */ .storageVolGetPath = esxStorageVolGetPath, /* 1.0.1 */ .connectListAllStoragePools = esxConnectListAllStoragePools, /* 5.10.0 */ + .storagePoolListAllVolumes = esxStoragePoolListAllVolumes, /* 5.10.0 */ }; diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index ab47a4c272..27f8b17e06 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -1566,6 +1566,90 @@ esxConnectListAllStoragePools(virConnectPtr conn,
+static int +esxStoragePoolListAllVolumes(virStoragePoolPtr pool, + virStorageVolPtr **vols, + unsigned int flags) +{ + bool success = false; + esxPrivate *priv = pool->conn->privateData; + esxVI_HostDatastoreBrowserSearchResults *searchResultsList = NULL; + esxVI_HostDatastoreBrowserSearchResults *searchResults = NULL; + esxVI_FileInfo *fileInfo = NULL; + char *directoryAndFileName = NULL; + size_t length; + size_t count = 0; + size_t i; + + virCheckFlags(0, -1); + + if (esxVI_LookupDatastoreContentByDatastoreName(priv->primary, pool->name, + &searchResultsList) < 0) { + goto cleanup; + } + + /* Interpret search result */ + for (searchResults = searchResultsList; searchResults; + searchResults = searchResults->_next) { + VIR_FREE(directoryAndFileName); + + if (esxUtil_ParseDatastorePath(searchResults->folderPath, NULL, NULL, + &directoryAndFileName) < 0) { + goto cleanup; + } + + /* Strip trailing separators */ + length = strlen(directoryAndFileName); + + while (length > 0 && directoryAndFileName[length - 1] == '/') { + directoryAndFileName[length - 1] = '\0'; + --length; + } + + /* Build volume names */ + for (fileInfo = searchResults->file; fileInfo; + fileInfo = fileInfo->_next) { + g_autofree char *datastorePath = NULL; + virStorageVolPtr volume; + + if (length < 1) { + datastorePath = g_strdup(fileInfo->path); + } else { + datastorePath = g_strdup_printf("%s/%s", + directoryAndFileName, + fileInfo->path); + } +
All this black magic shouldn't be duplicated IMO. I think it should be factored out first from esxStoragePoolListVolumes and then used here. All of Jano's previous comments apply to this one as well too - Cole
Signed-off-by: Pino Toscano <ptoscano@redhat.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index c362bf3a15..1718a686ce 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -55,6 +55,17 @@ </change> </section> <section title="Improvements"> + <change> + <summary> + esx: implement various APIs + </summary> + <description> + The <code>virConnectListAllStoragePools()</code>, + <code>virConnectListAllNetworks()</code> and + <code>virStoragePoolListAllVolumes()</code> APIs were implemented + in the esx driver. + </description> + </change> </section> <section title="Bug fixes"> </section> -- 2.21.0
participants (3)
-
Cole Robinson -
Ján Tomko -
Pino Toscano