Converting ENUMS to str can be user friendly though it can be problematic in matters of backward compatibility.
In particular when some ENUM in libvirt API will introduce a new constant, libvirt-dbus will fail with: size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative So using ints is preferable to avoid the previous issue. Signed-off-by: Katerina Koukiou <kkouk...@redhat.com> --- data/org.libvirt.Domain.xml | 14 ++-- src/domain.c | 172 ++++---------------------------------------- tests/test_domain.py | 6 +- 3 files changed, 25 insertions(+), 167 deletions(-) diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index db43b1c..6448a46 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -191,19 +191,19 @@ value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetBlockJobInfo"/> <arg name="disk" type="s" direction="in"/> <arg name="flags" type="u" direction="in"/> - <arg name="blockJobInfo" type="(sttt)" direction="out"/> + <arg name="blockJobInfo" type="(uttt)" direction="out"/> </method> <method name="GetControlInfo"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetControlInfo"/> <arg name="flags" type="u" direction="in"/> - <arg name="controlInfo" type="(sst)" direction="out"/> + <arg name="controlInfo" type="(uut)" direction="out"/> </method> <method name="GetDiskErrors"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetDiskErrors"/> <arg name="flags" type="u" direction="in"/> - <arg name="diskErrors" type="a(ss)" direction="out"/> + <arg name="diskErrors" type="a(su)" direction="out"/> </method> <method name="GetFSInfo"> <annotation name="org.gtk.GDBus.DocString" @@ -245,7 +245,7 @@ <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetJobStats"/> <arg name="flags" type="u" direction="in"/> - <arg name="stats" type="(sa{sv})" direction="out"/> + <arg name="stats" type="(ua{sv})" direction="out"/> </method> <method name="GetMemoryParameters"> <annotation name="org.gtk.GDBus.DocString" @@ -257,7 +257,7 @@ <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetMetadata Empty string can be used to pass a NULL as @uri argument."/> - <arg name="type" type="s" direction="in"/> + <arg name="type" type="u" direction="in"/> <arg name="uri" type="s" direction="in"/> <arg name="flags" type="u" direction="in"/> <arg name="metadata" type="s" direction="out"/> @@ -319,7 +319,7 @@ <method name="InterfaceAddresses"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainInterfaceAddresses"/> - <arg name="source" type="s" direction="in"/> + <arg name="source" type="u" direction="in"/> <arg name="flags" type="u" direction="in"/> <arg name="ifaces" type="a(ssa(isu))" direction="out"/> </method> @@ -492,7 +492,7 @@ <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetMetadata Empty string can be used to pass a NULL as @key or @uri argument."/> - <arg name="type" type="s" direction="in"/> + <arg name="type" type="u" direction="in"/> <arg name="metadata" type="s" direction="in"/> <arg name="key" type="s" direction="in"/> <arg name="uri" type="s" direction="in"/> diff --git a/src/domain.c b/src/domain.c index e305fa3..40cf2f7 100644 --- a/src/domain.c +++ b/src/domain.c @@ -3,75 +3,6 @@ #include <libvirt/libvirt.h> -VIRT_DBUS_ENUM_DECL(virtDBusDomainBlockJob) -VIRT_DBUS_ENUM_IMPL(virtDBusDomainBlockJob, - VIR_DOMAIN_BLOCK_JOB_TYPE_LAST, - "unknown", - "pull", - "copy", - "commit", - "active-commit") - -VIRT_DBUS_ENUM_DECL(virtDBusDomainControlErrorReason) -VIRT_DBUS_ENUM_IMPL(virtDBusDomainControlErrorReason, - VIR_DOMAIN_CONTROL_ERROR_REASON_LAST, - "none", - "unknown", - "monitor", - "internal") - -VIRT_DBUS_ENUM_DECL(virtDBusDomainControlState) -VIRT_DBUS_ENUM_IMPL(virtDBusDomainControlState, - VIR_DOMAIN_CONTROL_LAST, - "ok", - "job", - "occupied", - "error") - -VIRT_DBUS_ENUM_DECL(virtDBusDomainDiskError) -VIRT_DBUS_ENUM_IMPL(virtDBusDomainDiskError, - VIR_DOMAIN_DISK_ERROR_LAST, - "none", - "unspec", - "no-space") - -VIRT_DBUS_ENUM_DECL(virtDBusDomainInterfaceAddressesSource) -VIRT_DBUS_ENUM_IMPL(virtDBusDomainInterfaceAddressesSource, - VIR_DOMAIN_INTERFACE_ADDRESSES_SRC_LAST, - "lease", - "agent") - -VIRT_DBUS_ENUM_DECL(virtDBusDomainJob) -VIRT_DBUS_ENUM_IMPL(virtDBusDomainJob, - VIR_DOMAIN_JOB_LAST, - "none", - "bounded", - "unbounded", - "completed", - "failed", - "canceled") - -VIRT_DBUS_ENUM_DECL(virtDBusDomainMemoryStat) -VIRT_DBUS_ENUM_IMPL(virtDBusDomainMemoryStat, - VIR_DOMAIN_MEMORY_STAT_LAST, - "swap_in", - "swap_out", - "major_fault", - "minor_fault", - "unused", - "available", - "actual_baloon", - "rss", - "usable", - "last_update") - -VIRT_DBUS_ENUM_DECL(virtDBusDomainMetadata) -VIRT_DBUS_ENUM_IMPL(virtDBusDomainMetadata, - VIR_DOMAIN_METADATA_LAST, - "description", - "title", - "element") - struct _virtDBusDomainFSInfoList { virDomainFSInfoPtr *info; gint count; @@ -137,12 +68,8 @@ virtDBusDomainMemoryStatsToGVariant(virDomainMemoryStatPtr stats, g_variant_builder_init(&builder, G_VARIANT_TYPE("a{st}")); - for (gint i = 0; i < nr_stats; i++) { - const gchar *memoryStat = virtDBusDomainMemoryStatTypeToString(stats[i].tag); - if (!memoryStat) - continue; - g_variant_builder_add(&builder, "{st}", memoryStat, stats[i].val); - } + for (gint i = 0; i < nr_stats; i++) + g_variant_builder_add(&builder, "{ut}", stats[i].tag, stats[i].val); return g_variant_builder_end(&builder); } @@ -1032,7 +959,6 @@ virtDBusDomainGetBlockJobInfo(GVariant *inArgs, virDomainBlockJobInfo info; const gchar *disk; guint flags; - const gchar *blockJobTypeStr; g_variant_get(inArgs, "(&su)", &disk, &flags); @@ -1043,15 +969,7 @@ virtDBusDomainGetBlockJobInfo(GVariant *inArgs, if (virDomainGetBlockJobInfo(domain, disk, &info, flags) < 0) return virtDBusUtilSetLastVirtError(error); - blockJobTypeStr = virtDBusDomainBlockJobTypeToString(info.type); - if (!blockJobTypeStr) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't format virDomainBlockJobType '%d' to string.", - info.type); - return; - } - - *outArgs = g_variant_new("((sttt))", blockJobTypeStr, info.bandwidth, + *outArgs = g_variant_new("((uttt))", info.type, info.bandwidth, info.cur, info.end); } @@ -1067,8 +985,6 @@ virtDBusDomainGetControlInfo(GVariant *inArgs, virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; g_autofree virDomainControlInfoPtr controlInfo = NULL; - const gchar *stateStr; - const gchar *errorReasonStr; guint flags; g_variant_get(inArgs, "(u)", &flags); @@ -1081,23 +997,8 @@ virtDBusDomainGetControlInfo(GVariant *inArgs, if (virDomainGetControlInfo(domain, controlInfo, flags) < 0) return virtDBusUtilSetLastVirtError(error); - stateStr = virtDBusDomainControlStateTypeToString(controlInfo->state); - if (!stateStr) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't format virDomainControlState '%d' to string.", - controlInfo->state); - return; - } - errorReasonStr = virtDBusDomainControlErrorReasonTypeToString(controlInfo->details); - if (!errorReasonStr) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't format virDomainControlErrorReason '%d' to string.", - controlInfo->details); - return; - } - - *outArgs = g_variant_new("((sst))", stateStr, - errorReasonStr, controlInfo->stateTime); + *outArgs = g_variant_new("((uut))", controlInfo->state, + controlInfo->details, controlInfo->stateTime); } static void @@ -1137,15 +1038,11 @@ virtDBusDomainGetDiskErrors(GVariant *inArgs, return virtDBusUtilSetLastVirtError(error); } - g_variant_builder_init(&builder, G_VARIANT_TYPE("a(ss)")); + g_variant_builder_init(&builder, G_VARIANT_TYPE("a(su)")); for (gint i = 0; i < count; i++) { - const gchar *err = virtDBusDomainDiskErrorTypeToString(disks[i].error); - - if (!err) - continue; - g_variant_builder_open(&builder, G_VARIANT_TYPE("(ss)")); + g_variant_builder_open(&builder, G_VARIANT_TYPE("(su)")); g_variant_builder_add(&builder, "s", disks[i].disk); - g_variant_builder_add(&builder, "s", err); + g_variant_builder_add(&builder, "u", disks[i].error); g_variant_builder_close(&builder); } res = g_variant_builder_end(&builder); @@ -1355,7 +1252,6 @@ virtDBusDomainGetJobInfo(GVariant *inArgs G_GNUC_UNUSED, virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; g_autofree virDomainJobInfoPtr jobInfo = NULL; - const gchar *jobTypeStr; domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) @@ -1365,13 +1261,7 @@ virtDBusDomainGetJobInfo(GVariant *inArgs G_GNUC_UNUSED, if (virDomainGetJobInfo(domain, jobInfo) < 0) return virtDBusUtilSetLastVirtError(error); - jobTypeStr = virtDBusDomainJobTypeToString(jobInfo->type); - if (!jobTypeStr) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't format virDomainJobType '%d' to string.", jobInfo->type); - return; - } - *outArgs = g_variant_new("((sttttttttttt))", jobTypeStr, + *outArgs = g_variant_new("((uttttttttttt))", jobInfo->type, jobInfo->timeElapsed, jobInfo->timeRemaining, jobInfo->dataTotal, jobInfo->dataProcessed, jobInfo->dataRemaining, jobInfo->memTotal, @@ -1393,7 +1283,6 @@ virtDBusDomainGetJobStats(GVariant *inArgs, g_autoptr(virDomain) domain = NULL; g_auto(virtDBusUtilTypedParams) params = { 0 }; guint flags; - const gchar *typeStr; gint type; GVariant *grecords; GVariantBuilder builder; @@ -1412,15 +1301,8 @@ virtDBusDomainGetJobStats(GVariant *inArgs, grecords = virtDBusUtilTypedParamsToGVariant(params.params, params.nparams); - typeStr = virtDBusDomainJobTypeToString(type); - if (!typeStr) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't format virDomainJobType '%d' to string.", type); - return; - } - - g_variant_builder_init(&builder, G_VARIANT_TYPE("(sa{sv})")); - g_variant_builder_add(&builder, "s", typeStr); + g_variant_builder_init(&builder, G_VARIANT_TYPE("(ua{sv})")); + g_variant_builder_add(&builder, "u", type); g_variant_builder_add_value(&builder, grecords); gret = g_variant_builder_end(&builder); @@ -1474,13 +1356,12 @@ virtDBusDomainGetMetadata(GVariant *inArgs, { virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; - const gchar *typeStr; gint type; const gchar *uri; guint flags; g_autofree gchar *ret = NULL; - g_variant_get(inArgs, "(&s&su)", &typeStr, &uri, &flags); + g_variant_get(inArgs, "(u&su)", &type, &uri, &flags); if (g_str_equal(uri, "")) uri = NULL; @@ -1488,14 +1369,6 @@ virtDBusDomainGetMetadata(GVariant *inArgs, if (!domain) return; - type = virtDBusDomainMetadataTypeFromString(typeStr); - if (type < 0) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't get valid virDomainMetadataType from string '%s'.", - typeStr); - return; - } - ret = virDomainGetMetadata(domain, type, uri, flags); if (!ret) return virtDBusUtilSetLastVirtError(error); @@ -1787,25 +1660,17 @@ virtDBusDomainInterfaceAddresses(GVariant *inArgs, virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; gint source; - const gchar *sourceStr; g_auto(virtDBusDomainInterfaceList) ifaces = { 0 }; guint flags; GVariantBuilder builder; GVariant *res; - g_variant_get(inArgs, "(&su)", &sourceStr, &flags); + g_variant_get(inArgs, "(uu)", &source, &flags); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) return; - source = virtDBusDomainInterfaceAddressesSourceTypeFromString(sourceStr); - if (source < 0) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't get valid virDomainInterfaceAddressesSource from string '%s'.", - sourceStr); - return; - } ifaces.count = virDomainInterfaceAddresses(domain, &(ifaces.ifaces), source, flags); if (ifaces.count < 0) @@ -2617,14 +2482,13 @@ virtDBusDomainSetMetadata(GVariant *inArgs, { virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; - const gchar *typeStr; gint type; const gchar *metadata; const gchar *key; const gchar *uri; guint flags; - g_variant_get(inArgs, "(&s&s&s&su)", &typeStr, &metadata, &key, &uri, &flags); + g_variant_get(inArgs, "(u&s&s&su)", &type, &metadata, &key, &uri, &flags); if (g_str_equal(key, "")) key = NULL; if (g_str_equal(uri, "")) @@ -2634,14 +2498,6 @@ virtDBusDomainSetMetadata(GVariant *inArgs, if (!domain) return; - type = virtDBusDomainMetadataTypeFromString(typeStr); - if (type < 0) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't get valid virDomainMetadataType from string '%s'.", - typeStr); - return; - } - if (virDomainSetMetadata(domain, type, metadata, key, uri, flags) < 0) virtDBusUtilSetLastVirtError(error); } diff --git a/tests/test_domain.py b/tests/test_domain.py index d0cebcd..0e83f72 100755 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -64,10 +64,12 @@ class TestDomain(libvirttest.BaseTestClass): self.main_loop() def test_domain_metadata(self): + metadata_description = 0 obj, domain = self.domain() description_expected = "This is the Test domain" - domain.SetMetadata('description', description_expected, "", "", 0) - assert description_expected == domain.GetMetadata('description', "", 0) + domain.SetMetadata(metadata_description, + description_expected, "", "", 0) + assert description_expected == domain.GetMetadata(metadata_description, "", 0) def test_resume(self): def domain_resumed(path, _event): -- 2.15.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list