[libvirt] [dbus RFC PATCH 0/6] Remove all enum->string and string->enum code
As mentioned in the patches commit message... Converting ENUMS to str can be user friendly though it can be problematic between libvirt versions. In particular when some translated type will introduce a new constant to the ENUM libvirt-dbus will fail with: size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative Since it's not main use case of livirt-dbus API to be invoked interactively by user, having all ENUM types passed as unsigned int is preferable to avoid the previous issue. Katerina Koukiou (6): Abandon usage of all *TypeToString functions in domain.c Abandon usage of all *TypeToString functions in connect.c Abandon usage of all *TypeToString functions in network.c Change DomainEvent argument from string to unsigned int Change NetworkEvent argument from string to unsigned int Remove virtDBusUtilEnum{From,From}String functions data/org.libvirt.Connect.xml | 6 +- data/org.libvirt.Domain.xml | 14 ++-- data/org.libvirt.Network.xml | 6 +- src/connect.c | 18 +---- src/domain.c | 172 ++++--------------------------------------- src/events.c | 40 +--------- src/network.c | 66 +---------------- src/util.c | 27 ------- src/util.h | 28 ------- tests/libvirttest.py | 20 +++++ tests/test_connect.py | 24 ++++-- tests/test_domain.py | 36 ++++++--- tests/test_network.py | 20 +++-- 13 files changed, 107 insertions(+), 370 deletions(-) -- 2.15.0
Converting ENUMS to str can be user friendly though it can be problematic between libvirt versions. In particular when some translated type will introduce a new constant to the ENUM libvirt-dbus will fail with: size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative Since it's not main use case of livirt-dbus API to be invoked interactively by user, having all ENUM types passed as unsigned int is preferable to avoid the previous issue. Signed-off-by: Katerina Koukiou <kkoukiou@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
Converting ENUMS to str can be user friendly though it can be problematic between libvirt versions. In particular when some translated type will introduce a new constant to the ENUM libvirt-dbus will fail with: size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative Since it's not main use case of livirt-dbus API to be invoked interactively by user, having all ENUM types passed as unsigned int is preferable to avoid the previous issue. Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Connect.xml | 2 +- src/connect.c | 18 +----------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml index ee7bfdc..7249fa4 100644 --- a/data/org.libvirt.Connect.xml +++ b/data/org.libvirt.Connect.xml @@ -37,7 +37,7 @@ value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectCompareCPU"/> <arg name="xmlDesc" type="s" direction="in"/> <arg name="flags" type="u" direction="in"/> - <arg name="compareResult" type="s" direction="out"/> + <arg name="compareResult" type="u" direction="out"/> </method> <method name="DomainCreateXML"> <annotation name="org.gtk.GDBus.DocString" diff --git a/src/connect.c b/src/connect.c index 5e577e4..e21bfab 100644 --- a/src/connect.c +++ b/src/connect.c @@ -6,13 +6,6 @@ #include <glib/gprintf.h> -VIRT_DBUS_ENUM_DECL(virtDBusConnectCPUCompareResult) -VIRT_DBUS_ENUM_IMPL(virtDBusConnectCPUCompareResult, - VIR_CPU_COMPARE_LAST, - "incompatible", - "identical", - "superset") - static gint virtDBusConnectCredType[] = { VIR_CRED_AUTHNAME, VIR_CRED_ECHOPROMPT, @@ -263,7 +256,6 @@ virtDBusConnectCompareCPU(GVariant *inArgs, const gchar *xmlDesc; guint flags; gint compareResult; - const gchar* compareResultStr; g_variant_get(inArgs, "(&su)", &xmlDesc, &flags); @@ -274,15 +266,7 @@ virtDBusConnectCompareCPU(GVariant *inArgs, if (compareResult < 0) return virtDBusUtilSetLastVirtError(error); - compareResultStr = virtDBusConnectCPUCompareResultTypeToString(compareResult); - if (!compareResultStr) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't format virCPUCompareResult '%d' to string.", - compareResult); - return; - } - - *outArgs = g_variant_new("(s)", compareResultStr); + *outArgs = g_variant_new("(u)", compareResult); } static void -- 2.15.0
Converting ENUMS to str can be user friendly though it can be problematic between libvirt versions. In particular when some translated type will introduce a new constant to the ENUM libvirt-dbus will fail with: size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative Since it's not main use case of livirt-dbus API to be invoked interactively by user, having all ENUM types passed as unsigned int is preferable to avoid the previous issue. Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Network.xml | 6 ++-- src/network.c | 66 +++----------------------------------------- tests/test_network.py | 2 +- 3 files changed, 8 insertions(+), 66 deletions(-) diff --git a/data/org.libvirt.Network.xml b/data/org.libvirt.Network.xml index cf05062..e8c1506 100644 --- a/data/org.libvirt.Network.xml +++ b/data/org.libvirt.Network.xml @@ -43,7 +43,7 @@ Empty string will be returned in output for NULL variables."/> <arg name="mac" type="s" direction="in"/> <arg name="flags" type="u" direction="in"/> - <arg name="leases" type="a(stssssuss)" direction="out"/> + <arg name="leases" type="a(stusssuss)" direction="out"/> </method> <method name="GetXMLDesc"> <annotation name="org.gtk.GDBus.DocString" @@ -58,8 +58,8 @@ <method name="Update"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkUpdate"/> - <arg name="command" type="s" direction="in"/> - <arg name="section" type="s" direction="in"/> + <arg name="command" type="u" direction="in"/> + <arg name="section" type="u" direction="in"/> <arg name="parentIndex" type="i" direction="in"/> <arg name="xml" type="s" direction="in"/> <arg name="flags" type="u" direction="in"/> diff --git a/src/network.c b/src/network.c index 4d00dfe..256940f 100644 --- a/src/network.c +++ b/src/network.c @@ -3,38 +3,6 @@ #include <libvirt/libvirt.h> -VIRT_DBUS_ENUM_DECL(virtDBusNetworkIPAddr) -VIRT_DBUS_ENUM_IMPL(virtDBusNetworkIPAddr, - VIR_IP_ADDR_TYPE_LAST, - "ipv4", - "ipv6") - -VIRT_DBUS_ENUM_DECL(virtDBusNetworkUpdateCommand) -VIRT_DBUS_ENUM_IMPL(virtDBusNetworkUpdateCommand, - VIR_NETWORK_UPDATE_COMMAND_LAST, - "none", - "modify", - "delete", - "add-last", - "add-first") - -VIRT_DBUS_ENUM_DECL(virtDBusNetworkUpdateSection) -VIRT_DBUS_ENUM_IMPL(virtDBusNetworkUpdateSection, - VIR_NETWORK_SECTION_LAST, - "none", - "bridge", - "domain", - "ip", - "ip-dhcp-host", - "ip-dhcp-range", - "forward", - "forward-interface", - "forward-pf", - "portgroup", - "dns-host", - "dns-txt", - "dns-srv") - static void virtDBusNetworkDHCPLeaseListFree(virNetworkDHCPLeasePtr *leases) { @@ -284,20 +252,11 @@ virtDBusNetworkGetDHCPLeases(GVariant *inArgs, g_variant_builder_init(&builder, G_VARIANT_TYPE("a(stssssuss)")); for (gint i = 0; i < nleases; i++) { - const gchar *typeStr; - virNetworkDHCPLeasePtr lease = leases[i]; - typeStr = virtDBusNetworkIPAddrTypeToString(lease->type); - if (!typeStr) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't format virIPAddrType '%d' to string.", lease->type); - return; - } - - g_variant_builder_add(&builder, "(stssssuss)", + g_variant_builder_add(&builder, "(stusssuss)", lease->iface, lease->expirytime, - typeStr, lease->mac, + lease->type, lease->mac, lease->iaid ? lease->iaid : "" , lease->ipaddr, lease->prefix, lease->hostname ? lease->hostname : "", @@ -366,33 +325,16 @@ virtDBusNetworkUpdate(GVariant *inArgs, { virtDBusConnect *connect = userData; g_autoptr(virNetwork) network = NULL; - const gchar *commandStr; gint command; - const gchar *sectionStr; gint section; gint parentIndex; const gchar *xml; guint flags; - g_variant_get(inArgs, "(&s&si&su)", - &commandStr, §ionStr, + g_variant_get(inArgs, "(uui&su)", + &command, §ion, &parentIndex, &xml, &flags); - command = virtDBusNetworkUpdateCommandTypeFromString(commandStr); - if (command < 0) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't get valid virNetworkUpdateCommand from string '%s'.", - commandStr); - return; - } - section = virtDBusNetworkUpdateSectionTypeFromString(sectionStr); - if (section < 0) { - g_set_error(error, VIRT_DBUS_ERROR, VIRT_DBUS_ERROR_LIBVIRT, - "Can't get valid virNetworkUpdateSection from string '%s'.", - sectionStr); - return; - } - network = virtDBusNetworkGetVirNetwork(connect, objectPath, error); if (!network) return; diff --git a/tests/test_network.py b/tests/test_network.py index 2c1bd21..1340d95 100755 --- a/tests/test_network.py +++ b/tests/test_network.py @@ -80,7 +80,7 @@ class TestNetwork(libvirttest.BaseTestClass): self.main_loop() @pytest.mark.parametrize("command, section, parentIndex, xml_str, flags", [ - ('add-first', 'ip-dhcp-host', 0, ip_dhcp_host_xml, 0), + ('4', '4', 0, ip_dhcp_host_xml, 0), # add-first, ip-dhcp-host ]) def test_network_update(self, command, section, parentIndex, xml_str, flags): _, test_network = self.test_network() -- 2.15.0
Modify the relevant tests to comply with the new signal. Note: argument matching in connect_to_signal method is only usable with string and thus had to be refactored. Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Connect.xml | 2 +- src/events.c | 26 +------------------------- tests/libvirttest.py | 13 +++++++++++++ tests/test_connect.py | 12 ++++++++---- tests/test_domain.py | 30 ++++++++++++++++++++---------- 5 files changed, 43 insertions(+), 40 deletions(-) diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml index 7249fa4..386a7bb 100644 --- a/data/org.libvirt.Connect.xml +++ b/data/org.libvirt.Connect.xml @@ -170,7 +170,7 @@ <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectDomainEventCallback"/> <arg name="domain" type="o"/> - <arg name="event" type="s"/> + <arg name="event" type="u"/> </signal> <signal name="NetworkEvent"> <annotation name="org.gtk.GDBus.DocString" diff --git a/src/events.c b/src/events.c index 7e9da08..601d898 100644 --- a/src/events.c +++ b/src/events.c @@ -4,26 +4,6 @@ #include <libvirt/libvirt.h> -VIRT_DBUS_ENUM_DECL(virtDBusEventsDomainEvent) -VIRT_DBUS_ENUM_IMPL(virtDBusEventsDomainEvent, - VIR_DOMAIN_EVENT_LAST, - "Defined", - "Undefined", - "Started", - "Suspended", - "Resumed", - "Stopped", - "Shutdown", - "PMSuspended", - "Crashed") - -static const gchar * -virtDBusEventsDomainEventToString(gint event) -{ - const gchar *str = virtDBusEventsDomainEventTypeToString(event); - return str ? str : "unknown"; -} - static gint virtDBusEventsDomainLifecycle(virConnectPtr connection G_GNUC_UNUSED, virDomainPtr domain, @@ -33,10 +13,6 @@ virtDBusEventsDomainLifecycle(virConnectPtr connection G_GNUC_UNUSED, { virtDBusConnect *connect = opaque; g_autofree gchar *path = NULL; - const gchar *eventStr = virtDBusEventsDomainEventToString(event); - - if (!eventStr) - return 0; path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); @@ -45,7 +21,7 @@ virtDBusEventsDomainLifecycle(virConnectPtr connection G_GNUC_UNUSED, connect->connectPath, VIRT_DBUS_CONNECT_INTERFACE, "DomainEvent", - g_variant_new("(os)", path, eventStr), + g_variant_new("(ou)", path, event), NULL); return 0; diff --git a/tests/libvirttest.py b/tests/libvirttest.py index d1b71cc..73d51e6 100644 --- a/tests/libvirttest.py +++ b/tests/libvirttest.py @@ -1,3 +1,4 @@ +from enum import IntEnum from dbus.mainloop.glib import DBusGMainLoop from gi.repository import GLib import dbus @@ -18,6 +19,18 @@ def run(): exit(pytest.main(sys.argv)) +class DomainEvent(IntEnum): + DEFINED = 0 + UNDEFINED = 1 + STARTED = 2 + SUSPENDED = 3 + RESUMED = 4 + STOPPED = 5 + SHUTDOWN = 6 + PMSUSPENDED = 7 + CRASHED = 8 + + class BaseTestClass(): """ Base test class for whole test suite """ diff --git a/tests/test_connect.py b/tests/test_connect.py index 57f0658..41cc134 100755 --- a/tests/test_connect.py +++ b/tests/test_connect.py @@ -31,11 +31,13 @@ class TestConnect(libvirttest.BaseTestClass): ''' def test_connect_domain_create_xml(self): - def domain_started(path, _event): + def domain_started(path, event): + if event != libvirttest.DomainEvent.STARTED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('DomainEvent', domain_started, arg1='Started') + self.connect.connect_to_signal('DomainEvent', domain_started) path = self.connect.DomainCreateXML(self.minimal_domain_xml, 0) assert isinstance(path, dbus.ObjectPath) @@ -43,11 +45,13 @@ class TestConnect(libvirttest.BaseTestClass): self.main_loop() def test_comnect_domain_define_xml(self): - def domain_defined(path, _event): + def domain_defined(path, event): + if event != libvirttest.DomainEvent.DEFINED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('DomainEvent', domain_defined, arg1='Defined') + self.connect.connect_to_signal('DomainEvent', domain_defined) path = self.connect.DomainDefineXML(self.minimal_domain_xml) assert isinstance(path, dbus.ObjectPath) diff --git a/tests/test_domain.py b/tests/test_domain.py index 0e83f72..2def6c1 100755 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -47,11 +47,13 @@ class TestDomain(libvirttest.BaseTestClass): assert autostart_current == dbus.Boolean(autostart_expected) def test_domain_managed_save(self): - def domain_stopped(path, _event): + def domain_stopped(path, event): + if event != libvirttest.DomainEvent.STOPPED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('DomainEvent', domain_stopped, arg1='Stopped') + self.connect.connect_to_signal('DomainEvent', domain_stopped) obj, domain = self.domain() domain.ManagedSave(0) @@ -72,11 +74,13 @@ class TestDomain(libvirttest.BaseTestClass): assert description_expected == domain.GetMetadata(metadata_description, "", 0) def test_resume(self): - def domain_resumed(path, _event): + def domain_resumed(path, event): + if event != libvirttest.DomainEvent.RESUMED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('DomainEvent', domain_resumed, arg1='Resumed') + self.connect.connect_to_signal('DomainEvent', domain_resumed) obj, domain = self.domain() domain.Suspend() @@ -88,11 +92,13 @@ class TestDomain(libvirttest.BaseTestClass): self.main_loop() def test_shutdown(self): - def domain_stopped(path, _event): + def domain_stopped(path, event): + if event != libvirttest.DomainEvent.STOPPED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('DomainEvent', domain_stopped, arg1='Stopped') + self.connect.connect_to_signal('DomainEvent', domain_stopped) obj, domain = self.domain() domain.Shutdown(0) @@ -103,11 +109,13 @@ class TestDomain(libvirttest.BaseTestClass): self.main_loop() def test_suspend(self): - def domain_suspended(path, _event): + def domain_suspended(path, event): + if event != libvirttest.DomainEvent.SUSPENDED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('DomainEvent', domain_suspended, arg1='Suspended') + self.connect.connect_to_signal('DomainEvent', domain_suspended) obj, domain = self.domain() domain.Suspend() @@ -118,11 +126,13 @@ class TestDomain(libvirttest.BaseTestClass): self.main_loop() def test_undefine(self): - def domain_undefined(path, _event): + def domain_undefined(path, event): + if event != libvirttest.DomainEvent.UNDEFINED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('DomainEvent', domain_undefined, arg1='Undefined') + self.connect.connect_to_signal('DomainEvent', domain_undefined) _, domain = self.domain() domain.Shutdown(0) -- 2.15.0
Modify the relevant tests to comply with the new signal. Note: argument matching in connect_to_signal method is only usable with string and thus had to be refactored. Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Connect.xml | 2 +- src/events.c | 14 +------------- tests/libvirttest.py | 7 +++++++ tests/test_connect.py | 12 ++++++++---- tests/test_network.py | 18 ++++++++++++------ 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml index 386a7bb..8272da6 100644 --- a/data/org.libvirt.Connect.xml +++ b/data/org.libvirt.Connect.xml @@ -176,7 +176,7 @@ <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-network.html#virConnectNetworkEventLifecycleCallback"/> <arg name="network" type="o"/> - <arg name="event" type="s"/> + <arg name="event" type="u"/> </signal> </interface> </node> diff --git a/src/events.c b/src/events.c index 601d898..1b584f7 100644 --- a/src/events.c +++ b/src/events.c @@ -146,14 +146,6 @@ virtDBusEventsDomainDiskChange(virConnectPtr connection G_GNUC_UNUSED, return 0; } -VIRT_DBUS_ENUM_DECL(virtDBusEventsNetworkEvent) -VIRT_DBUS_ENUM_IMPL(virtDBusEventsNetworkEvent, - VIR_NETWORK_EVENT_LAST, - "Defined", - "Undefined", - "Started", - "Stopped") - static gint virtDBusEventsNetworkLifecycle(virConnectPtr connection G_GNUC_UNUSED, virNetworkPtr network, @@ -163,10 +155,6 @@ virtDBusEventsNetworkLifecycle(virConnectPtr connection G_GNUC_UNUSED, { virtDBusConnect *connect = opaque; g_autofree gchar *path = NULL; - const gchar *eventStr = virtDBusEventsNetworkEventTypeToString(event); - - if (!eventStr) - return 0; path = virtDBusUtilBusPathForVirNetwork(network, connect->networkPath); @@ -175,7 +163,7 @@ virtDBusEventsNetworkLifecycle(virConnectPtr connection G_GNUC_UNUSED, connect->connectPath, VIRT_DBUS_CONNECT_INTERFACE, "NetworkEvent", - g_variant_new("(os)", path, eventStr), + g_variant_new("(ou)", path, event), NULL); return 0; diff --git a/tests/libvirttest.py b/tests/libvirttest.py index 73d51e6..fc3ed5e 100644 --- a/tests/libvirttest.py +++ b/tests/libvirttest.py @@ -31,6 +31,13 @@ class DomainEvent(IntEnum): CRASHED = 8 +class NetworkEvent(IntEnum): + DEFINED = 0 + UNDEFINED = 1 + STARTED = 2 + STOPPED = 3 + + class BaseTestClass(): """ Base test class for whole test suite """ diff --git a/tests/test_connect.py b/tests/test_connect.py index 41cc134..7748822 100755 --- a/tests/test_connect.py +++ b/tests/test_connect.py @@ -125,11 +125,13 @@ class TestConnect(libvirttest.BaseTestClass): assert isinstance(self.connect.GetCPUModelNames(arch, 0), dbus.Array) def test_connect_network_create_xml(self): - def network_started(path, _event): + def network_started(path, event): + if event != libvirttest.NetworkEvent.STARTED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('NetworkEvent', network_started, arg1='Started') + self.connect.connect_to_signal('NetworkEvent', network_started) path = self.connect.NetworkCreateXML(self.minimal_network_xml) assert isinstance(path, dbus.ObjectPath) @@ -137,11 +139,13 @@ class TestConnect(libvirttest.BaseTestClass): self.main_loop() def test_connect_network_define_xml(self): - def network_defined(path, _event): + def network_defined(path, event): + if event != libvirttest.NetworkEvent.DEFINED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('NetworkEvent', network_defined, arg1='Defined') + self.connect.connect_to_signal('NetworkEvent', network_defined) path = self.connect.NetworkDefineXML(self.minimal_network_xml) assert isinstance(path, dbus.ObjectPath) diff --git a/tests/test_network.py b/tests/test_network.py index 1340d95..f34c081 100755 --- a/tests/test_network.py +++ b/tests/test_network.py @@ -34,11 +34,13 @@ class TestNetwork(libvirttest.BaseTestClass): assert autostart_current == dbus.Boolean(autostart_expected) def test_network_create(self): - def domain_started(path, _event): + def domain_started(path, event): + if event != libvirttest.NetworkEvent.STARTED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('NetworkEvent', domain_started, arg1='Started') + self.connect.connect_to_signal('NetworkEvent', domain_started) _,test_network = self.test_network() interface_obj = dbus.Interface(test_network, 'org.libvirt.Network') @@ -48,11 +50,13 @@ class TestNetwork(libvirttest.BaseTestClass): self.main_loop() def test_network_destroy(self): - def network_stopped(path, _event): + def network_stopped(path, event): + if event != libvirttest.NetworkEvent.STOPPED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('NetworkEvent', network_stopped, arg1='Stopped') + self.connect.connect_to_signal('NetworkEvent', network_stopped) _, test_network = self.test_network() interface_obj = dbus.Interface(test_network, 'org.libvirt.Network') @@ -66,11 +70,13 @@ class TestNetwork(libvirttest.BaseTestClass): assert isinstance(interface_obj.GetXMLDesc(0), dbus.String) def test_network_undefine(self): - def domain_undefined(path, _event): + def domain_undefined(path, event): + if event != libvirttest.NetworkEvent.UNDEFINED: + return assert isinstance(path, dbus.ObjectPath) self.loop.quit() - self.connect.connect_to_signal('NetworkEvent', domain_undefined, arg1='Undefined') + self.connect.connect_to_signal('NetworkEvent', domain_undefined) _,test_network = self.test_network() interface_obj = dbus.Interface(test_network, 'org.libvirt.Network') -- 2.15.0
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- src/util.c | 27 --------------------------- src/util.h | 28 ---------------------------- 2 files changed, 55 deletions(-) diff --git a/src/util.c b/src/util.c index a9d130e..4efa3ec 100644 --- a/src/util.c +++ b/src/util.c @@ -214,33 +214,6 @@ virtDBusUtilVirDomainListFree(virDomainPtr *domains) g_free(domains); } -const gchar * -virtDBusUtilEnumToString(const gchar *const *types, - guint ntypes, - gint type) -{ - if (type < 0 || (guint)type >= ntypes) - return NULL; - - return types[type]; -} - -gint -virtDBusUtilEnumFromString(const gchar *const *types, - guint ntypes, - const gchar *type) -{ - guint i; - if (!type) - return -1; - - for (i = 0; i < ntypes; i++) - if (g_str_equal(types[i], type)) - return i; - - return -1; -} - virNetworkPtr virtDBusUtilVirNetworkFromBusPath(virConnectPtr connection, const gchar *path, diff --git a/src/util.h b/src/util.h index 4a2138a..3309803 100644 --- a/src/util.h +++ b/src/util.h @@ -57,34 +57,6 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainPtr, virtDBusUtilVirDomainListFree); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainStatsRecordPtr, virDomainStatsRecordListFree); -gint -virtDBusUtilEnumFromString(const gchar *const *types, - guint ntypes, - const gchar *type) G_GNUC_PURE; - -const gchar * -virtDBusUtilEnumToString(const gchar *const *types, - guint ntypes, - gint type) G_GNUC_PURE; - -#define VIRT_DBUS_ENUM_IMPL(name, lastVal, ...) \ - static const gchar *const name ##TypeList[] = { __VA_ARGS__ }; \ - G_STATIC_ASSERT(G_N_ELEMENTS(name ##TypeList) == lastVal); \ - const gchar *name ##TypeToString(gint type) { \ - return virtDBusUtilEnumToString(name ##TypeList, \ - G_N_ELEMENTS(name ##TypeList), \ - type); \ - } \ - gint name ##TypeFromString(const gchar *type) { \ - return virtDBusUtilEnumFromString(name ##TypeList, \ - G_N_ELEMENTS(name ##TypeList), \ - type); \ - } - -#define VIRT_DBUS_ENUM_DECL(name) \ - const gchar *name ##TypeToString(gint type) G_GNUC_PURE; \ - gint name ##TypeFromString(const gchar *type) G_GNUC_PURE; - virNetworkPtr virtDBusUtilVirNetworkFromBusPath(virConnectPtr connection, const gchar *path, -- 2.15.0
On Thu, 2018-05-03 at 10:45 +0200, Katerina Koukiou wrote:
As mentioned in the patches commit message...
Converting ENUMS to str can be user friendly though it can be problematic between libvirt versions.
In particular when some translated type will introduce a new constant to the ENUM libvirt-dbus will fail with:
size of array ‘_GStaticAssertCompileTimeAssertion_5’ is negative
Since it's not main use case of livirt-dbus API to be invoked interactively by user, having all ENUM types passed as unsigned int is preferable to avoid the previous issue.
Katerina Koukiou (6): Abandon usage of all *TypeToString functions in domain.c Abandon usage of all *TypeToString functions in connect.c Abandon usage of all *TypeToString functions in network.c Change DomainEvent argument from string to unsigned int Change NetworkEvent argument from string to unsigned int Remove virtDBusUtilEnum{From,From}String functions
data/org.libvirt.Connect.xml | 6 +- data/org.libvirt.Domain.xml | 14 ++-- data/org.libvirt.Network.xml | 6 +- src/connect.c | 18 +---- src/domain.c | 172 ++++----------------------------- ---------- src/events.c | 40 +--------- src/network.c | 66 +---------------- src/util.c | 27 ------- src/util.h | 28 ------- tests/libvirttest.py | 20 +++++ tests/test_connect.py | 24 ++++-- tests/test_domain.py | 36 ++++++--- tests/test_network.py | 20 +++-- 13 files changed, 107 insertions(+), 370 deletions(-)
There are two more occurrences of translation of ENUM to String. virDBusDomainGetState and virtDBusEventsDomainTrayChange. If there are not objections with this change I 'll repost with these two fixed. Katerina
participants (1)
-
Katerina Koukiou