[libvirt] [dbus PATCH v2 0/9] Remove all enum->string and string->enum code

Changes from v1: * Added the three last patches Katerina Koukiou (9): 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 Remove reason to string translation in virtDBusEventsDomainTrayChange Remove state to string translation in virtDBusDomainGetState Remove reason to string translation in virtDBusEventsDomainDiskChange data/org.libvirt.Connect.xml | 6 +- data/org.libvirt.Domain.xml | 20 ++--- data/org.libvirt.Network.xml | 6 +- src/connect.c | 18 +--- src/domain.c | 203 ++++--------------------------------------- src/events.c | 72 ++------------- src/network.c | 66 +------------- src/util.c | 27 ------ src/util.h | 28 ------ tests/libvirttest.py | 32 +++++++ tests/test_connect.py | 24 +++-- tests/test_domain.py | 46 ++++++---- tests/test_network.py | 20 +++-- 13 files changed, 131 insertions(+), 437 deletions(-) -- 2.15.0

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 <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

On Fri, May 04, 2018 at 10:38:27AM +0200, Katerina Koukiou wrote:
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 <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/src/domain.c b/src/domain.c index e305fa3..40cf2f7 100644 --- a/src/domain.c +++ b/src/domain.c
[...]
@@ -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);
MemoryStats method needs to be updated in the introspection file.
return 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,
GetJobInfo method needs to be updated in the introspection file.
jobInfo->timeElapsed, jobInfo->timeRemaining, jobInfo->dataTotal, jobInfo->dataProcessed, jobInfo->dataRemaining, jobInfo->memTotal,
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

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 <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

On Fri, May 04, 2018 at 10:38:28AM +0200, Katerina Koukiou wrote:
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 <kkoukiou@redhat.com> --- data/org.libvirt.Connect.xml | 2 +- src/connect.c | 18 +----------------- 2 files changed, 2 insertions(+), 18 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

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 <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

On Fri, May 04, 2018 at 10:38:29AM +0200, Katerina Koukiou wrote:
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 <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(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

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..f7f48ed 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 @@ -85,3 +86,15 @@ class BaseTestClass(): path = self.connect.ListNetworks(0)[0] obj = self.bus.get_object('org.libvirt', path) return path, obj + + +class DomainEvent(IntEnum): + DEFINED = 0 + UNDEFINED = 1 + STARTED = 2 + SUSPENDED = 3 + RESUMED = 4 + STOPPED = 5 + SHUTDOWN = 6 + PMSUSPENDED = 7 + CRASHED = 8 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

On Fri, May 04, 2018 at 10:38:30AM +0200, Katerina Koukiou wrote:
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(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

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 f7f48ed..0e84a94 100644 --- a/tests/libvirttest.py +++ b/tests/libvirttest.py @@ -98,3 +98,10 @@ class DomainEvent(IntEnum): SHUTDOWN = 6 PMSUSPENDED = 7 CRASHED = 8 + + +class NetworkEvent(IntEnum): + DEFINED = 0 + UNDEFINED = 1 + STARTED = 2 + STOPPED = 3 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

On Fri, May 04, 2018 at 10:38:31AM +0200, Katerina Koukiou wrote:
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(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

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 Fri, May 04, 2018 at 10:38:32AM +0200, Katerina Koukiou wrote:
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- src/util.c | 27 --------------------------- src/util.h | 28 ---------------------------- 2 files changed, 55 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 2 +- src/events.c | 15 +-------------- 2 files changed, 2 insertions(+), 15 deletions(-) diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index 6448a46..98c018c 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -578,7 +578,7 @@ <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectDomainEventTrayChangeCallback"/> <arg name="device" type="s"/> - <arg name="reason" type="s"/> + <arg name="reason" type="u"/> </signal> </interface> </node> diff --git a/src/events.c b/src/events.c index 1b584f7..d4c7145 100644 --- a/src/events.c +++ b/src/events.c @@ -80,28 +80,15 @@ virtDBusEventsDomainTrayChange(virConnectPtr connection G_GNUC_UNUSED, { virtDBusConnect *connect = opaque; g_autofree gchar *path = NULL; - const gchar *reasonstr; path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); - switch (reason) { - case VIR_DOMAIN_EVENT_TRAY_CHANGE_OPEN: - reasonstr = "open"; - break; - case VIR_DOMAIN_EVENT_TRAY_CHANGE_CLOSE: - reasonstr = "close"; - break; - default: - reasonstr = ""; - break; - } - g_dbus_connection_emit_signal(connect->bus, NULL, path, VIRT_DBUS_DOMAIN_INTERFACE, "TrayChange", - g_variant_new("(ss)", device, reasonstr), + g_variant_new("(su)", device, reason), NULL); return 0; -- 2.15.0

On Fri, May 04, 2018 at 10:38:33AM +0200, Katerina Koukiou wrote:
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 2 +- src/events.c | 15 +-------------- 2 files changed, 2 insertions(+), 15 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Adjust tests to comply with the new type. Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 2 +- src/domain.c | 31 +------------------------------ tests/libvirttest.py | 12 ++++++++++++ tests/test_domain.py | 10 +++++----- 4 files changed, 19 insertions(+), 36 deletions(-) diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index 98c018c..3627b1b 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -32,7 +32,7 @@ <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetSchedulerType"/> </property> - <property name="State" type="s" access="read"> + <property name="State" type="u" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetState"/> </property> diff --git a/src/domain.c b/src/domain.c index 40cf2f7..059cd3e 100644 --- a/src/domain.c +++ b/src/domain.c @@ -251,7 +251,6 @@ virtDBusDomainGetState(const gchar *objectPath, virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; gint state = 0; - const gchar *string; domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) @@ -260,35 +259,7 @@ virtDBusDomainGetState(const gchar *objectPath, if (virDomainGetState(domain, &state, NULL, 0) < 0) return virtDBusUtilSetLastVirtError(error); - switch (state) { - case VIR_DOMAIN_NOSTATE: - default: - string = "nostate"; - break; - case VIR_DOMAIN_RUNNING: - string = "running"; - break; - case VIR_DOMAIN_BLOCKED: - string = "blocked"; - break; - case VIR_DOMAIN_PAUSED: - string = "paused"; - break; - case VIR_DOMAIN_SHUTDOWN: - string = "shutdown"; - break; - case VIR_DOMAIN_SHUTOFF: - string = "shutoff"; - break; - case VIR_DOMAIN_CRASHED: - string = "crashed"; - break; - case VIR_DOMAIN_PMSUSPENDED: - string = "pmsuspended"; - break; - } - - *value = g_variant_new("s", string); + *value = g_variant_new("u", state); } static void diff --git a/tests/libvirttest.py b/tests/libvirttest.py index 0e84a94..06ac0e4 100644 --- a/tests/libvirttest.py +++ b/tests/libvirttest.py @@ -100,6 +100,18 @@ class DomainEvent(IntEnum): CRASHED = 8 +class DomainState(IntEnum): + NOSTATE = 0 + RUNNING = 1 + BLOCKED = 2 + PAUSED = 3 + SHUTDOWN = 4 + SHUTOFF = 5 + CRASHED = 6 + PMSUSPENDED = 7 + LAST = 8 + + class NetworkEvent(IntEnum): DEFINED = 0 UNDEFINED = 1 diff --git a/tests/test_domain.py b/tests/test_domain.py index 2def6c1..c7e09cd 100755 --- a/tests/test_domain.py +++ b/tests/test_domain.py @@ -19,7 +19,7 @@ class TestDomain(libvirttest.BaseTestClass): assert any([isinstance(props['SchedulerType'], dbus.Struct), isinstance(props['SchedulerType'][0], dbus.String), isinstance(props['SchedulerType'][1], dbus.Int32)]) - assert isinstance(props['State'], dbus.String) + assert isinstance(props['State'], dbus.UInt32) assert isinstance(props['Updated'], dbus.Boolean) assert isinstance(props['UUID'], dbus.String) @@ -59,7 +59,7 @@ class TestDomain(libvirttest.BaseTestClass): domain.ManagedSave(0) assert domain.HasManagedSaveImage(0) == dbus.Boolean(True) state = obj.Get('org.libvirt.Domain', 'State', dbus_interface=dbus.PROPERTIES_IFACE) - assert state == 'shutoff' + assert state == libvirttest.DomainState.SHUTOFF domain.ManagedSaveRemove(0) assert domain.HasManagedSaveImage(0) == dbus.Boolean(False) @@ -87,7 +87,7 @@ class TestDomain(libvirttest.BaseTestClass): domain.Resume() state = obj.Get('org.libvirt.Domain', 'State', dbus_interface=dbus.PROPERTIES_IFACE) - assert state == 'running' + assert state == libvirttest.DomainState.RUNNING self.main_loop() @@ -104,7 +104,7 @@ class TestDomain(libvirttest.BaseTestClass): domain.Shutdown(0) state = obj.Get('org.libvirt.Domain', 'State', dbus_interface=dbus.PROPERTIES_IFACE) - assert state == 'shutoff' + assert state == libvirttest.DomainState.SHUTOFF self.main_loop() @@ -121,7 +121,7 @@ class TestDomain(libvirttest.BaseTestClass): domain.Suspend() state = obj.Get('org.libvirt.Domain', 'State', dbus_interface=dbus.PROPERTIES_IFACE) - assert state == 'paused' + assert state == libvirttest.DomainState.PAUSED self.main_loop() -- 2.15.0

On Fri, May 04, 2018 at 10:38:34AM +0200, Katerina Koukiou wrote:
Adjust tests to comply with the new type.
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 2 +- src/domain.c | 31 +------------------------------ tests/libvirttest.py | 12 ++++++++++++ tests/test_domain.py | 10 +++++----- 4 files changed, 19 insertions(+), 36 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>

Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 2 +- src/events.c | 17 ++--------------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index 3627b1b..a09e868 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -572,7 +572,7 @@ <arg name="oldSrcPath" type="s"/> <arg name="newSrcPath" type="s"/> <arg name="device" type="s"/> - <arg name="reason" type="s"/> + <arg name="reason" type="u"/> </signal> <signal name="TrayChange"> <annotation name="org.gtk.GDBus.DocString" diff --git a/src/events.c b/src/events.c index d4c7145..b432535 100644 --- a/src/events.c +++ b/src/events.c @@ -105,29 +105,16 @@ virtDBusEventsDomainDiskChange(virConnectPtr connection G_GNUC_UNUSED, { virtDBusConnect *connect = opaque; g_autofree gchar *path = NULL; - const gchar *reasonstr; path = virtDBusUtilBusPathForVirDomain(domain, connect->domainPath); - switch (reason) { - case VIR_DOMAIN_EVENT_DISK_CHANGE_MISSING_ON_START: - reasonstr = "missing-on-start"; - break; - case VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START: - reasonstr = "missing-on-start"; - break; - default: - reasonstr = ""; - break; - } - g_dbus_connection_emit_signal(connect->bus, NULL, path, VIRT_DBUS_DOMAIN_INTERFACE, "DiskChange", - g_variant_new("(ssss)", old_src_path, - new_src_path, device, reasonstr), + g_variant_new("(sssu)", old_src_path, + new_src_path, device, reason), NULL); return 0; -- 2.15.0

On Fri, May 04, 2018 at 10:38:35AM +0200, Katerina Koukiou wrote:
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 2 +- src/events.c | 17 ++--------------- 2 files changed, 3 insertions(+), 16 deletions(-)
Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Katerina Koukiou
-
Pavel Hrdina