On Thu, Oct 10, 2019 at 01:43:09PM +0200, Simon Kobyda wrote: > Signed-off-by: Simon Kobyda <skobyda@xxxxxxxxxx> > --- > data/org.libvirt.DomainSnapshot.xml | 7 +++ > src/connect.c | 6 +++ > src/connect.h | 1 + > src/domainsnapshot.c | 79 +++++++++++++++++++++++++++++ > src/domainsnapshot.h | 9 ++++ > src/meson.build | 1 + > src/util.c | 49 ++++++++++++++++++ > src/util.h | 16 ++++++ > 8 files changed, 168 insertions(+) > create mode 100644 data/org.libvirt.DomainSnapshot.xml > create mode 100644 src/domainsnapshot.c > create mode 100644 src/domainsnapshot.h > > diff --git a/data/org.libvirt.DomainSnapshot.xml b/data/org.libvirt.DomainSnapshot.xml > new file mode 100644 > index 0000000..80f61c6 > --- /dev/null > +++ b/data/org.libvirt.DomainSnapshot.xml > @@ -0,0 +1,7 @@ > +<!DOCTYPE node PUBLIC "-//freedesktop//DTD D-BUS Object Introspection 1.0//EN" > +"http://www.freedesktop.org/standards/dbus/1.0/introspect.dtd";> > + > +<node name="/org/libvirt/snapshot"> I would rather name it as domainsnapshot. It's unlikely that we will ever have some other type of snapshot in libvirt but it will at least follow the same naming as in libvirt APIs. > + <interface name="org.libvirt.DomainSnapshot"> > + </interface> > +</node> > diff --git a/src/connect.c b/src/connect.c > index f8f76a2..b40b0ba 100644 > --- a/src/connect.c > +++ b/src/connect.c > @@ -1,5 +1,6 @@ > #include "connect.h" > #include "domain.h" > +#include "domainsnapshot.h" > #include "events.h" > #include "interface.h" > #include "network.h" > @@ -2002,6 +2003,7 @@ virtDBusConnectFree(virtDBusConnect *connect) > virtDBusConnectClose(connect, TRUE); > > g_free(connect->domainPath); > + g_free(connect->domainSnapshotPath); > g_free(connect->interfacePath); > g_free(connect->networkPath); > g_free(connect->nodeDevPath); > @@ -2062,6 +2064,10 @@ virtDBusConnectNew(virtDBusConnect **connectp, > if (error && *error) > return; > > + virtDBusDomainSnapshotRegister(connect, error); > + if (error && *error) > + return; > + > virtDBusInterfaceRegister(connect, error); > if (error && *error) > return; > diff --git a/src/connect.h b/src/connect.h > index 829bd57..74c89cf 100644 > --- a/src/connect.h > +++ b/src/connect.h > @@ -13,6 +13,7 @@ struct virtDBusConnect { > const gchar *uri; > const gchar *connectPath; > gchar *domainPath; > + gchar *domainSnapshotPath; > gchar *interfacePath; > gchar *networkPath; > gchar *nodeDevPath; > diff --git a/src/domainsnapshot.c b/src/domainsnapshot.c > new file mode 100644 > index 0000000..590cbef > --- /dev/null > +++ b/src/domainsnapshot.c > @@ -0,0 +1,79 @@ > +#include "domainsnapshot.h" > +#include "util.h" > + > +#include <libvirt/libvirt.h> > + > +static virtDBusGDBusPropertyTable virtDBusDomainSnapshotPropertyTable[] = { > + { 0 } > +}; > + > +static virtDBusGDBusMethodTable virtDBusDomainSnapshotMethodTable[] = { > + { 0 } > +}; > + > +static gchar ** > +virtDBusDomainSnapshotEnumerate(gpointer userData) > +{ > + virtDBusConnect *connect = userData; > + g_autoptr(virDomainPtr) domains = NULL; > + gint numDoms = 0; > + GPtrArray *list = NULL; There is no need to use GPtrArray. > + > + if (!virtDBusConnectOpen(connect, NULL)) > + return NULL; > + > + numDoms = virConnectListAllDomains(connect->connection, &domains, 0); > + if (numDoms <= 0) > + return NULL; Here you can allocate the return array if strings directly ... > + > + list = g_ptr_array_new(); > + > + for (gint i = 0; i < numDoms; i++) { > + g_autoptr(virDomainSnapshotPtr) domainSnapshots = NULL; > + gint numSnaps; > + > + numSnaps = virDomainListAllSnapshots(domains[i], &domainSnapshots, 0); > + if (numSnaps <= 0) > + continue; > + > + for (gint j = 0; j < numSnaps; j++) { > + gchar *snapPath = virtDBusUtilBusPathForVirDomainSnapshot(domains[i], > + domainSnapshots[j], > + connect->domainSnapshotPath); > + g_ptr_array_add(list, snapPath); ... and here you can just add the path to the array. In addition the list that you are using is not used after this point. > + } > + } > + > + gchar **ret = g_new0(gchar *, numDoms + 1); > + > + for (gint i = 0; i < numDoms; i++) { > + ret[i] = virtDBusUtilBusPathForVirDomain(domains[i], > + connect->domainPath); > + } Here you call new for loop that will fill the ret value with domain paths instead of domainSnapshot paths. > + > + return ret; > +} > + > +static GDBusInterfaceInfo *interfaceInfo = NULL; > + > +void > +virtDBusDomainSnapshotRegister(virtDBusConnect *connect, > + GError **error) > +{ > + connect->domainSnapshotPath = g_strdup_printf("%s/snapshot", connect->connectPath); > + > + if (!interfaceInfo) { > + interfaceInfo = virtDBusGDBusLoadIntrospectData(VIRT_DBUS_DOMAIN_SNAPSHOT_INTERFACE, > + error); > + if (!interfaceInfo) > + return; > + } > + > + virtDBusGDBusRegisterSubtree(connect->bus, > + connect->domainSnapshotPath, > + interfaceInfo, > + virtDBusDomainSnapshotEnumerate, > + virtDBusDomainSnapshotMethodTable, > + virtDBusDomainSnapshotPropertyTable, > + connect); > +} > diff --git a/src/domainsnapshot.h b/src/domainsnapshot.h > new file mode 100644 > index 0000000..7d8a938 > --- /dev/null > +++ b/src/domainsnapshot.h > @@ -0,0 +1,9 @@ > +#pragma once > + > +#include "connect.h" > + > +#define VIRT_DBUS_DOMAIN_SNAPSHOT_INTERFACE "org.libvirt.DomainSnapshot" > + > +void > +virtDBusDomainSnapshotRegister(virtDBusConnect *connect, > + GError **error); > diff --git a/src/meson.build b/src/meson.build > index de96596..1839c79 100644 > --- a/src/meson.build > +++ b/src/meson.build > @@ -16,6 +16,7 @@ exe_libvirt_dbus = executable( > [ > 'connect.c', > 'domain.c', > + 'domainsnapshot.c', > 'events.c', > 'gdbus.c', > 'interface.c', > diff --git a/src/util.c b/src/util.c > index 103bb29..daf3c2a 100644 > --- a/src/util.c > +++ b/src/util.c > @@ -279,6 +279,55 @@ virtDBusUtilVirDomainListFree(virDomainPtr *domains) > g_free(domains); > } > > +virDomainSnapshotPtr > +virtDBusUtilVirDomainSnapshotFromBusPath(virConnectPtr connection, > + const gchar *path, > + const gchar *domainSnapshotPath) > +{ > + g_autofree gchar *uuidStr = NULL; > + g_autofree gchar *snapshotName = NULL; > + g_autoptr(virDomain) domain = NULL; > + gsize prefixLen = strlen(domainSnapshotPath) + 1; > + gchar** strings = g_strsplit(path + prefixLen, "_", 2); > + > + uuidStr = virtDBusUtilDecodeStr(strings[0]); > + snapshotName = virtDBusUtilDecodeStr(strings[1]); > + > + domain = virDomainLookupByUUIDString(connection, uuidStr); > + > + return virDomainSnapshotLookupByName(domain, snapshotName, 0); > +} > + > +gchar * > +virtDBusUtilBusPathForVirDomainSnapshot(virDomainPtr domain, > + virDomainSnapshotPtr domainSnapshot, > + const gchar *domainSnapshotPath) > +{ > + const gchar *snapshotName = NULL; > + gchar uuidStr[VIR_UUID_STRING_BUFLEN] = ""; > + g_autofree const gchar *encodedDomainNameStr = NULL; > + g_autofree const gchar *encodedSnapshotNameStr = NULL; > + > + if (virDomainGetUUIDString(domain, uuidStr) < 0) > + return virtDBusUtilSetLastVirtError(error); This check is pointless since the libvirt function virDomainGetUUIDString will never fail, only programming error can make it fail and that's not our case. In addition this function fails to compile as there is no variable error. > + > + snapshotName = virDomainSnapshotGetName(domainSnapshot); > + encodedDomainNameStr = virtDBusUtilEncodeStr(uuidStr); Here you should call virtDBusUtilEncodeUUID as uuid is encoded differently. This would work but is not necessary. > + encodedSnapshotNameStr = virtDBusUtilEncodeStr(snapshotName); > + > + return g_strdup_printf("%s/%s_%s", domainSnapshotPath, > + encodedDomainNameStr, encodedSnapshotNameStr); I'm not sure if single '_' is a good separator as that char is used to prefix hex value of encoded characters. For example uuid 00000000-1111-2222-3333-444444444444 is encoded into _00000000_1111_2222_3333_444444444444 by virtDBusUtilEncodeUUID or into 00000000_2d1111_2d2222_2d3333_2d444444444444 by virtDBusUtilEncodeStr As you can see the function virtDBusUtilVirDomainSnapshotFromBusPath whould fail to properly split the string into domainUUID part and snapshotName part in both cases. We need to figure out a different separator. To be on a safe side we can create a macro that will hold a length of the encoded domain UUID which will be always the same and simply us that length to split the domainSnapshotPath into domainUUID and snapshotName. Let's keep the formatting string as "%s/%s_%s" and in the function virtDBusUtilVirDomainSnapshotFromBusPath you can simply do this: /* result of strlen("_00000000_1111_2222_3333_444444444444") */ #define VIRT_DBUS_UUID_LEN = 37 ... virtDBusUtilVirDomainSnapshotFromBusPath(...) { ... g_autofree gchar *tmp = g_strdup(path + prefixLen); tmp[VIRT_DBUS_UUID_LEN] = 0; uuidStr = virtDBusUtilDecodeUUID(tmp); snapshotName = virtDBusUtilDecodeStr(tmp + VIRT_DBUS_UUID_LEN + 1) ... } If the path is like this: "_00000000_1111_2222_3333_444444444444_my_snapshot" It will replace the '_' after the UUID by 0 and you will get two separate strings. Pavel > +} > + > +void > +virtDBusUtilVirDomainSnapshotListFree(virDomainSnapshotPtr* domainSnapshots) > +{ > + for (gint i = 0; domainSnapshots[i] != NULL; i++) > + virDomainSnapshotFree(domainSnapshots[i]); > + > + g_free(domainSnapshots); > +} > + > virInterfacePtr > virtDBusUtilVirInterfaceFromBusPath(virConnectPtr connection, > const gchar *path, > diff --git a/src/util.h b/src/util.h > index b05c2fc..c0b314d 100644 > --- a/src/util.h > +++ b/src/util.h > @@ -65,6 +65,22 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainPtr, virtDBusUtilVirDomainListFree); > > G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainStatsRecordPtr, virDomainStatsRecordListFree); > > +virDomainSnapshotPtr > +virtDBusUtilVirDomainSnapshotFromBusPath(virConnectPtr connection, > + const gchar *path, > + const gchar *domainSnapshotPath); > + > +gchar * > +virtDBusUtilBusPathForVirDomainSnapshot(virDomainPtr domain, > + virDomainSnapshotPtr domainSnapshot, > + const gchar *domainSnapshotPath); > + > +void > +virtDBusUtilVirDomainSnapshotListFree(virDomainSnapshotPtr* domainSnapshots); > + > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSnapshot, virDomainSnapshotFree); > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSnapshotPtr, virtDBusUtilVirDomainSnapshotListFree); > + > virInterfacePtr > virtDBusUtilVirInterfaceFromBusPath(virConnectPtr connection, > const gchar *path, > -- > 2.21.0 > > -- > libvir-list mailing list > libvir-list@xxxxxxxxxx > https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
