[dbus PATCH v2 1/9] Abandon usage of all *TypeToString functions in domain.c

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
---
 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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux