[libvirt] [PATCH v2 04/12] esx: implement connectListAllStoragePools
Ján Tomko
jtomko at redhat.com
Tue Dec 3 15:17:57 UTC 2019
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 at 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 at redhat.com>
Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191203/9f8dfd26/attachment-0001.sig>
More information about the libvir-list
mailing list