[libvirt] [dbus PATCH v2 1/9] Abandon usage of all *TypeToString functions in domain.c
Katerina Koukiou
kkoukiou at redhat.com
Fri May 4 08:38:27 UTC 2018
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 at 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
More information about the libvir-list
mailing list