[libvirt] [PATCH 2/2] storage: add support for new rbd_list2 method

Daniel P. Berrangé posted 2 patches 6 years, 10 months ago
[libvirt] [PATCH 2/2] storage: add support for new rbd_list2 method
Posted by Daniel P. Berrangé 6 years, 10 months ago
The rbd_list method has been deprecated in Ceph >= 14.0.0
in favour of the new rbd_list2 method which populates an
array of structs.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 m4/virt-storage-rbd.m4            |  1 +
 src/storage/storage_backend_rbd.c | 32 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/m4/virt-storage-rbd.m4 b/m4/virt-storage-rbd.m4
index 17e2115309..f3d9d04908 100644
--- a/m4/virt-storage-rbd.m4
+++ b/m4/virt-storage-rbd.m4
@@ -33,6 +33,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_RBD], [
       old_LIBS="$LIBS"
       LIBS="$LIBS $LIBRBD_LIBS"
       AC_CHECK_FUNCS([rbd_get_features],[],[LIBRBD_FOUND=no])
+      AC_CHECK_FUNCS([rbd_list2])
       LIBS="$old_LIBS"
     fi
 
diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
index 24ddbc8f69..b6f2785390 100644
--- a/src/storage/storage_backend_rbd.c
+++ b/src/storage/storage_backend_rbd.c
@@ -572,6 +572,33 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr)
     char **names = NULL;
     size_t nnames = 0;
     int ret;
+#ifdef HAVE_RBD_LIST2
+    rbd_image_spec_t *images = NULL;
+    size_t nimages = 16;
+    size_t i;
+
+    while (true) {
+        if (VIR_ALLOC_N(images, nimages) < 0)
+            goto error;
+
+        ret = rbd_list2(ptr->ioctx, images, &nimages);
+        if (ret >= 0)
+            break;
+        if (ret != -ERANGE) {
+            virReportSystemError(-ret, "%s", _("Unable to list RBD images"));
+            goto error;
+        }
+    }
+
+    if (VIR_ALLOC_N(names, nimages + 1) < 0)
+        goto error;
+    nnames = nimages;
+
+    for (i = 0; i < nimages; i++) {
+        names[i] = images->name;
+        images->name = NULL;
+    }
+#else  /* ! HAVE_RBD_LIST2 */
     size_t max_size = 1024;
     VIR_AUTOFREE(char *) namebuf = NULL;
     const char *name;
@@ -607,11 +634,16 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr)
 
     if (VIR_EXPAND_N(names, nnames, 1) < 0)
         goto error;
+#endif /* ! HAVE_RBD_LIST2 */
 
     return names;
 
  error:
     virStringListFreeCount(names, nnames);
+#ifdef HAVE_RBD_LIST2
+    rbd_image_spec_list_cleanup(images, nimages);
+    VIR_FREE(images);
+#endif /* ! HAVE_RBD_LIST2 */
     return NULL;
 }
 
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] storage: add support for new rbd_list2 method
Posted by Ján Tomko 6 years, 10 months ago
On Mon, Mar 18, 2019 at 12:20:18PM +0000, Daniel P. Berrangé wrote:
>The rbd_list method has been deprecated in Ceph >= 14.0.0
>in favour of the new rbd_list2 method which populates an
>array of structs.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> m4/virt-storage-rbd.m4            |  1 +
> src/storage/storage_backend_rbd.c | 32 +++++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
>
>diff --git a/m4/virt-storage-rbd.m4 b/m4/virt-storage-rbd.m4
>index 17e2115309..f3d9d04908 100644
>--- a/m4/virt-storage-rbd.m4
>+++ b/m4/virt-storage-rbd.m4
>@@ -33,6 +33,7 @@ AC_DEFUN([LIBVIRT_STORAGE_CHECK_RBD], [
>       old_LIBS="$LIBS"
>       LIBS="$LIBS $LIBRBD_LIBS"
>       AC_CHECK_FUNCS([rbd_get_features],[],[LIBRBD_FOUND=no])
>+      AC_CHECK_FUNCS([rbd_list2])
>       LIBS="$old_LIBS"
>     fi
>
>diff --git a/src/storage/storage_backend_rbd.c b/src/storage/storage_backend_rbd.c
>index 24ddbc8f69..b6f2785390 100644
>--- a/src/storage/storage_backend_rbd.c
>+++ b/src/storage/storage_backend_rbd.c
>@@ -572,6 +572,33 @@ virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr)
>     char **names = NULL;
>     size_t nnames = 0;
>     int ret;
>+#ifdef HAVE_RBD_LIST2

Please, create two separate implementations:

#ifdef HAVE_RBD_LIST2
static char **
virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr)
{
    ...
}
#else /* ! HAVE_RBD_LIST2 */
virStorageBackendRBDGetVolNames(virStorageBackendRBDStatePtr ptr)
{
    ...
}
#endif

A bit more verbose, but easier to read.

>+    rbd_image_spec_t *images = NULL;
>+    size_t nimages = 16;
>+    size_t i;
>+
>+    while (true) {
>+        if (VIR_ALLOC_N(images, nimages) < 0)
>+            goto error;
>+
>+        ret = rbd_list2(ptr->ioctx, images, &nimages);
>+        if (ret >= 0)
>+            break;
>+        if (ret != -ERANGE) {
>+            virReportSystemError(-ret, "%s", _("Unable to list RBD images"));
>+            goto error;
>+        }
>+    }
>+
>+    if (VIR_ALLOC_N(names, nimages + 1) < 0)
>+        goto error;
>+    nnames = nimages;
>+
>+    for (i = 0; i < nimages; i++) {
>+        names[i] = images->name;
>+        images->name = NULL;

This can also use VIR_STEAL_PTR

>+    }
>+#else  /* ! HAVE_RBD_LIST2 */
>     size_t max_size = 1024;
>     VIR_AUTOFREE(char *) namebuf = NULL;

Reviewed-by: Ján Tomko <jtomko@redhat.com>

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