[libvirt] [dbus PATCH 01/15] Introduce StorageVol Interface

Ján Tomko jtomko at redhat.com
Wed Jun 13 14:21:33 UTC 2018


On Tue, Jun 12, 2018 at 11:00:14AM +0200, Katerina Koukiou wrote:
>Signed-off-by: Katerina Koukiou <kkoukiou at redhat.com>
>---
> data/Makefile.am                |  3 +-
> data/org.libvirt.StorageVol.xml |  7 +++
> src/Makefile.am                 |  3 +-
> src/connect.c                   |  6 +++
> src/connect.h                   |  1 +
> src/storagevol.c                | 96 +++++++++++++++++++++++++++++++++++++++++
> src/storagevol.h                |  9 ++++
> src/util.c                      | 35 +++++++++++++++
> src/util.h                      | 16 +++++++
> 9 files changed, 174 insertions(+), 2 deletions(-)
> create mode 100644 data/org.libvirt.StorageVol.xml
> create mode 100644 src/storagevol.c
> create mode 100644 src/storagevol.h
>
>diff --git a/data/Makefile.am b/data/Makefile.am
>index fdec857..5688cab 100644
>--- a/data/Makefile.am
>+++ b/data/Makefile.am
>@@ -24,7 +24,8 @@ interfaces_files = \
> 	org.libvirt.Network.xml \
>         org.libvirt.NWFilter.xml \
> 	org.libvirt.Secret.xml \
>-	org.libvirt.StoragePool.xml
>+	org.libvirt.StoragePool.xml \
>+	org.libvirt.StorageVol.xml

org.libvirt.StorageVol.xml \
$(NULL)

That way all the future additions can include the trailing slash,
making for nicer diffs.

> interfacesdir = $(DBUS_INTERFACES_DIR)
> interfaces_DATA = $(interfaces_files)
>

>diff --git a/src/Makefile.am b/src/Makefile.am
>index 22128c2..539e722 100644
>--- a/src/Makefile.am
>+++ b/src/Makefile.am
>@@ -12,7 +12,8 @@ DAEMON_SOURCES = \
> 	network.c network.h \
> 	nwfilter.c nwfilter.h \
> 	secret.c secret.h \
>-	storagepool.c storagepool.h
>+	storagepool.c storagepool.h \
>+	storagevol.c storagevol.h

$(NULL) here as well

>
> EXTRA_DIST = \
> 	$(DAEMON_SOURCES)

>diff --git a/src/storagevol.c b/src/storagevol.c
>new file mode 100644
>index 0000000..dad7d11
>--- /dev/null
>+++ b/src/storagevol.c
>@@ -0,0 +1,96 @@
>+#include "storagevol.h"
>+#include "util.h"
>+
>+#include <libvirt/libvirt.h>
>+
>+static virtDBusGDBusPropertyTable virtDBusStorageVolPropertyTable[] = {
>+    { 0 }
>+};
>+
>+static virtDBusGDBusMethodTable virtDBusStorageVolMethodTable[] = {
>+    { 0 }
>+};
>+
>+static gchar **
>+virtDBusStorageVolEnumerate(gpointer userData)
>+{
>+    virtDBusConnect *connect = userData;
>+    g_autoptr(virStoragePoolPtr) storagePools = NULL;
>+    gint numPools = 0;
>+    gint numVols = 0;
>+    gint numVolsTotal = 0;
>+    gint offset = 0;
>+    gchar **ret = NULL;
>+
>+    if (!virtDBusConnectOpen(connect, NULL))
>+        return NULL;
>+
>+    numPools = virConnectListAllStoragePools(connect->connection,
>+                                             &storagePools, 0);
>+    if (numPools < 0)
>+        return NULL;
>+
>+    if (numPools == 0)
>+        return NULL;
>+
>+    for (gint i = 0; i < numPools; i++) {
>+        g_autoptr(virStorageVolPtr) storageVols = NULL;
>+
>+        numVols = virStoragePoolListAllVolumes(storagePools[i],
>+                                               &storageVols, 0);
>+        if (numVols < 0)
>+            return NULL;
>+
>+        if (numVols == 0)
>+            return NULL;
>+
>+        numVolsTotal += numVols;
>+    }
>+
>+    ret = g_new0(gchar *, numVolsTotal + 1);
>+
>+    for (gint i = 0; i < numPools; i++) {
>+        g_autoptr(virStorageVolPtr) storageVols = NULL;
>+
>+        numVols = virStoragePoolListAllVolumes(storagePools[i],
>+                                               &storageVols, 0);

We don't need to list the volumes twice, not only it seems inefficient,
the number of volumes can change in the meantime, leading to possible
invalid memory access.

Possibly an APPEND_ELEMENT macro similar to what we have in libvirt,
so that we can test it separately in test_util.c.

>+        if (numVols < 0)
>+            return NULL;

This would leak ret.

>+
>+        if (numVols == 0)
>+            return NULL;
>+
>+        for (gint j = 0; j < numVols; j++) {
>+            ret[offset] = virtDBusUtilBusPathForVirStorageVol(storageVols[j],
>+                                                              connect->storageVolPath);
>+            offset++;
>+        }
>+    }
>+
>+    return ret;
>+}
>+

>diff --git a/src/util.c b/src/util.c
>index 1268736..1d11ce6 100644
>--- a/src/util.c
>+++ b/src/util.c
>@@ -417,3 +417,38 @@ virtDBusUtilVirStoragePoolListFree(virStoragePoolPtr *storagePools)
>
>     g_free(storagePools);
> }
>+
>+virStorageVolPtr
>+virtDBusUtilVirStorageVolFromBusPath(virConnectPtr connection,
>+                                     const gchar *path,
>+                                     const gchar *storageVolPath)
>+{
>+    g_autofree gchar *key = NULL;
>+    gsize prefixLen = strlen(storageVolPath);
>+
>+    key = virtDBusUtilDecodeStr(path + prefixLen + 1);

In other cases, the + 1 is a part of the prefixLen.

>+
>+    return virStorageVolLookupByKey(connection, key);
>+}
>+

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180613/fc46bd4e/attachment-0001.sig>


More information about the libvir-list mailing list