Re: [PATCH v2 04/12] esx: implement connectListAllStoragePools

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
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@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux