[libvirt] [dbus PATCH v2 0/6] Use API calls supporting flags
* Use API calls with flags parameter when possible * Since libvirt-dbus API doesn't follow strictly the naming of libvirt API there should be documentation regarding which libvirt API is used for each libvirt-dbus API. This patch introduces documentation annotations in D-Bus Interface XML files which will be used later to generate proper documentation. Katerina Koukiou (6): virtDBusDomainGetVcpus: Should be implemented as method and not as property virtDBusDomainShutdown: Use virDomainShutdownFlags instead of virDomainShutdown virtDBusDomainCreate: Use virDomainCreateWithFlags instead of virDomainCreate virtDBusDomainUndefine: Use virDomainUndefineFlags instead of virDomainUndefine virtDBusDomainDestroy: Use virDomainDestroyFlags instead of virDomainDestroy Use Documentation Annotations in D-Bus Interface XML data/org.libvirt.Connect.xml | 24 ++++++++++++ data/org.libvirt.Domain.xml | 87 +++++++++++++++++++++++++++++++++++++------- src/domain.c | 71 +++++++++++++++++++++++------------- test/test_domain.py | 22 +++++++---- 4 files changed, 157 insertions(+), 47 deletions(-) -- 2.15.0
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 5 ++++- src/domain.c | 51 ++++++++++++++++++++++++++------------------- test/test_domain.py | 3 ++- 3 files changed, 35 insertions(+), 24 deletions(-) diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index 48bf40f..1ecf826 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -6,12 +6,15 @@ <property name="Name" type="s" access="read"/> <property name="UUID" type="s" access="read"/> <property name="Id" type="u" access="read"/> - <property name="Vcpus" type="u" access="read"/> <property name="OSType" type="s" access="read"/> <property name="Active" type="b" access="read"/> <property name="Persistent" type="b" access="read"/> <property name="State" type="s" access="read"/> <property name="Autostart" type="b" access="read"/> + <method name="GetVcpus"> + <arg name="flags" type="u" direction="in"/> + <arg name="vcpus" type="u" direction="out"/> + </method> <method name="GetXMLDesc"> <arg name="flags" type="u" direction="in"/> <arg name="xml" type="s" direction="out"/> diff --git a/src/domain.c b/src/domain.c index e4404c1..09b3440 100644 --- a/src/domain.c +++ b/src/domain.c @@ -86,27 +86,6 @@ virtDBusDomainGetId(const gchar *objectPath, *value = g_variant_new("u", id); } -static void -virtDBusDomainGetVcpus(const gchar *objectPath, - gpointer userData, - GVariant **value, - GError **error) -{ - virtDBusConnect *connect = userData; - g_autoptr(virDomain) domain = NULL; - gint vcpus; - - domain = virtDBusDomainGetVirDomain(connect, objectPath, error); - if (!domain) - return; - - vcpus = virDomainGetVcpusFlags(domain, VIR_DOMAIN_VCPU_CURRENT); - if (vcpus < 0) - return virtDBusUtilSetLastVirtError(error); - - *value = g_variant_new("u", vcpus); -} - static void virtDBusDomainGetOsType(const gchar *objectPath, gpointer userData, @@ -239,6 +218,34 @@ virtDBusDomainGetAutostart(const gchar *objectPath, *value = g_variant_new("b", !!autostart); } +static void +virtDBusDomainGetVcpus(GVariant *inArgs, + GUnixFDList *inFDs G_GNUC_UNUSED, + const gchar *objectPath, + gpointer userData, + GVariant **outArgs, + GUnixFDList **outFDs G_GNUC_UNUSED, + GError **error) + +{ + virtDBusConnect *connect = userData; + g_autoptr(virDomain) domain = NULL; + gint vcpus; + guint flags; + + g_variant_get(inArgs, "(u)", &flags); + + domain = virtDBusDomainGetVirDomain(connect, objectPath, error); + if (!domain) + return; + + vcpus = virDomainGetVcpusFlags(domain, flags); + if (vcpus < 0) + return virtDBusUtilSetLastVirtError(error); + + *outArgs = g_variant_new("(u)", vcpus); +} + static void virtDBusDomainGetXMLDesc(GVariant *inArgs, GUnixFDList *inFDs G_GNUC_UNUSED, @@ -435,7 +442,6 @@ static virtDBusGDBusPropertyTable virtDBusDomainPropertyTable[] = { { "Name", virtDBusDomainGetName, NULL }, { "UUID", virtDBusDomainGetUUID, NULL }, { "Id", virtDBusDomainGetId, NULL }, - { "Vcpus", virtDBusDomainGetVcpus, NULL }, { "OSType", virtDBusDomainGetOsType, NULL }, { "Active", virtDBusDomainGetActive, NULL }, { "Persistent", virtDBusDomainGetPersistent, NULL }, @@ -445,6 +451,7 @@ static virtDBusGDBusPropertyTable virtDBusDomainPropertyTable[] = { }; static virtDBusGDBusMethodTable virtDBusDomainMethodTable[] = { + { "GetVcpus", virtDBusDomainGetVcpus }, { "GetXMLDesc", virtDBusDomainGetXMLDesc }, { "GetStats", virtDBusDomainGetStats }, { "Shutdown", virtDBusDomainShutdown }, diff --git a/test/test_domain.py b/test/test_domain.py index 1bf9c1b..22039dc 100755 --- a/test/test_domain.py +++ b/test/test_domain.py @@ -17,7 +17,6 @@ class TestDomain(libvirttest.BaseTestClass): assert isinstance(props['Name'], dbus.String) assert isinstance(props['UUID'], dbus.String) assert isinstance(props['Id'], dbus.UInt32) - assert isinstance(props['Vcpus'], dbus.UInt32) assert isinstance(props['OSType'], dbus.String) assert isinstance(props['Active'], dbus.Boolean) assert isinstance(props['Persistent'], dbus.Boolean) @@ -29,6 +28,8 @@ class TestDomain(libvirttest.BaseTestClass): xml = domain.GetXMLDesc(0) assert isinstance(xml, dbus.String) + vcpus = domain.GetVcpus(0) + assert isinstance(vcpus, dbus.UInt32) domain.Reboot(0) domain.Shutdown() -- 2.15.0
On Fri, Mar 23, 2018 at 05:57:59PM +0100, Katerina Koukiou wrote:
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 5 ++++- src/domain.c | 51 ++++++++++++++++++++++++++------------------- test/test_domain.py | 3 ++- 3 files changed, 35 insertions(+), 24 deletions(-)
The $subject can be shorten to "virtDBusDomainGetVcpus: Implement as method not as property" Pavel
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 4 +++- src/domain.c | 5 ++++- test/test_domain.py | 6 +++--- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index 1ecf826..692948c 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -24,7 +24,9 @@ <arg name="flags" type="u" direction="in"/> <arg name="records" type="a{sv}" direction="out"/> </method> - <method name="Shutdown"/> + <method name="Shutdown"> + <arg name="flags" type="u" direction="in"/> + </method> <method name="Destroy"/> <method name="Reboot"> <arg name="flags" type="u" direction="in"/> diff --git a/src/domain.c b/src/domain.c index 09b3440..8b821a2 100644 --- a/src/domain.c +++ b/src/domain.c @@ -321,12 +321,15 @@ virtDBusDomainShutdown(GVariant *inArgs G_GNUC_UNUSED, { virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; + guint flags; + + g_variant_get(inArgs, "(u)", &flags); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) return; - if (virDomainShutdown(domain) < 0) + if (virDomainShutdownFlags(domain, flags) < 0) virtDBusUtilSetLastVirtError(error); } diff --git a/test/test_domain.py b/test/test_domain.py index 22039dc..18a1434 100755 --- a/test/test_domain.py +++ b/test/test_domain.py @@ -32,7 +32,7 @@ class TestDomain(libvirttest.BaseTestClass): assert isinstance(vcpus, dbus.UInt32) domain.Reboot(0) - domain.Shutdown() + domain.Shutdown(0) domain.Create() domain.Destroy() domain.Undefine() @@ -46,7 +46,7 @@ class TestDomain(libvirttest.BaseTestClass): self.connect.connect_to_signal('DomainStopped', domain_stopped) obj, domain = self.domain() - domain.Shutdown() + domain.Shutdown(0) state = obj.Get('org.libvirt.Domain', 'State', dbus_interface=dbus.PROPERTIES_IFACE) assert state == 'shutoff' @@ -62,7 +62,7 @@ class TestDomain(libvirttest.BaseTestClass): self.connect.connect_to_signal('DomainUndefined', domain_undefined) _, domain = self.domain() - domain.Shutdown() + domain.Shutdown(0) domain.Undefine() self.main_loop() -- 2.15.0
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 4 +++- src/domain.c | 5 ++++- test/test_domain.py | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index 692948c..8208374 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -34,7 +34,9 @@ <method name="Reset"> <arg name="flags" type="u" direction="in"/> </method> - <method name="Create"/> + <method name="Create"> + <arg name="flags" type="u" direction="in"/> + </method> <method name="Undefine"/> <signal name="DeviceAdded"> <arg name="device" type="s"/> diff --git a/src/domain.c b/src/domain.c index 8b821a2..b5f80f8 100644 --- a/src/domain.c +++ b/src/domain.c @@ -412,12 +412,15 @@ virtDBusDomainCreate(GVariant *inArgs G_GNUC_UNUSED, { virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; + guint flags; + + g_variant_get(inArgs, "(u)", &flags); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) return; - if (virDomainCreate(domain) < 0) + if (virDomainCreateWithFlags(domain, flags) < 0) virtDBusUtilSetLastVirtError(error); } diff --git a/test/test_domain.py b/test/test_domain.py index 18a1434..0291159 100755 --- a/test/test_domain.py +++ b/test/test_domain.py @@ -33,7 +33,7 @@ class TestDomain(libvirttest.BaseTestClass): domain.Reboot(0) domain.Shutdown(0) - domain.Create() + domain.Create(0) domain.Destroy() domain.Undefine() -- 2.15.0
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 4 +++- src/domain.c | 5 ++++- test/test_domain.py | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index 8208374..187f604 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -37,7 +37,9 @@ <method name="Create"> <arg name="flags" type="u" direction="in"/> </method> - <method name="Undefine"/> + <method name="Undefine"> + <arg name="flags" type="u" direction="in"/> + </method> <signal name="DeviceAdded"> <arg name="device" type="s"/> </signal> diff --git a/src/domain.c b/src/domain.c index b5f80f8..fd6609c 100644 --- a/src/domain.c +++ b/src/domain.c @@ -435,12 +435,15 @@ virtDBusDomainUndefine(GVariant *inArgs G_GNUC_UNUSED, { virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; + guint flags; + + g_variant_get(inArgs, "(u)", &flags); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) return; - if (virDomainUndefine(domain) < 0) + if (virDomainUndefineFlags(domain, flags) < 0) virtDBusUtilSetLastVirtError(error); } diff --git a/test/test_domain.py b/test/test_domain.py index 0291159..8cd0d6a 100755 --- a/test/test_domain.py +++ b/test/test_domain.py @@ -35,7 +35,7 @@ class TestDomain(libvirttest.BaseTestClass): domain.Shutdown(0) domain.Create(0) domain.Destroy() - domain.Undefine() + domain.Undefine(0) def test_shutdown(self): def domain_stopped(name, path): @@ -63,7 +63,7 @@ class TestDomain(libvirttest.BaseTestClass): _, domain = self.domain() domain.Shutdown(0) - domain.Undefine() + domain.Undefine(0) self.main_loop() -- 2.15.0
We need to catch Exceptions when testing this API call, since virDomainDestroyFlags will be supported in test-driver in 4.2.0 libvirt release. Better version checks for unsupported API calls need to be done in all tests, but it's not relevant to this commit. When virConnectGetVersion will be supported we can move to better implementation, and python logging should be used to provide proper warnings for skipped tests. Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Domain.xml | 4 +++- src/domain.c | 5 ++++- test/test_domain.py | 7 ++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index 187f604..e79fb5e 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -27,7 +27,9 @@ <method name="Shutdown"> <arg name="flags" type="u" direction="in"/> </method> - <method name="Destroy"/> + <method name="Destroy"> + <arg name="flags" type="u" direction="in"/> + </method> <method name="Reboot"> <arg name="flags" type="u" direction="in"/> </method> diff --git a/src/domain.c b/src/domain.c index fd6609c..3c09351 100644 --- a/src/domain.c +++ b/src/domain.c @@ -344,12 +344,15 @@ virtDBusDomainDestroy(GVariant *inArgs G_GNUC_UNUSED, { virtDBusConnect *connect = userData; g_autoptr(virDomain) domain = NULL; + guint flags; + + g_variant_get(inArgs, "(u)", &flags); domain = virtDBusDomainGetVirDomain(connect, objectPath, error); if (!domain) return; - if (virDomainDestroy(domain) < 0) + if (virDomainDestroyFlags(domain, flags) < 0) virtDBusUtilSetLastVirtError(error); } diff --git a/test/test_domain.py b/test/test_domain.py index 8cd0d6a..73737d7 100755 --- a/test/test_domain.py +++ b/test/test_domain.py @@ -3,6 +3,7 @@ import dbus import libvirttest +DBUS_EXCEPTION_MISSING_FUNCTION = 'this function is not supported by the connection driver' class TestDomain(libvirttest.BaseTestClass): def domain(self): @@ -34,7 +35,11 @@ class TestDomain(libvirttest.BaseTestClass): domain.Reboot(0) domain.Shutdown(0) domain.Create(0) - domain.Destroy() + try: + domain.Destroy(0) + except dbus.exceptions.DBusException as e: + if not any(DBUS_EXCEPTION_MISSING_FUNCTION in arg for arg in e.args): + raise e domain.Undefine(0) def test_shutdown(self): -- 2.15.0
On Fri, Mar 23, 2018 at 05:58:03PM +0100, Katerina Koukiou wrote:
We need to catch Exceptions when testing this API call, since virDomainDestroyFlags will be supported in test-driver in 4.2.0 libvirt release.
Better version checks for unsupported API calls need to be done in all tests, but it's not relevant to this commit. When virConnectGetVersion will be supported we can move to better implementation, and python logging should be used to provide proper warnings for skipped tests.
Long lines in commit message, the unwritten rule (suggestion) is to keep it under 72 characters per line. Pavel
Since we don't follow the exact naming of libvirt API for libvirt-dbus, documentation should clarify which API call is used internally each time. Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Connect.xml | 24 ++++++++++++++++ data/org.libvirt.Domain.xml | 66 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 82 insertions(+), 8 deletions(-) diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml index 787cd8d..9849abe 100644 --- a/data/org.libvirt.Connect.xml +++ b/data/org.libvirt.Connect.xml @@ -4,51 +4,75 @@ <node name="/org/libvirt/connect"> <interface name="org.libvirt.Connect"> <method name="ListDomains"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectListDomains"/> <arg name="flags" type="u" direction="in"/> <arg name="domains" type="ao" direction="out"/> </method> <method name="CreateXML"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCreateXML"/> <arg name="xml" type="s" direction="in"/> <arg name="flags" type="u" direction="in"/> <arg name="domain" type="o" direction="out"/> </method> <method name="DefineXML"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDefineXML"/> <arg name="xml" type="s" direction="in"/> <arg name="domain" type="o" direction="out"/> </method> <signal name="DomainCrashed"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_CRASHED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainDefined"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_DEFINED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainPMSuspended"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_SUSPENDED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainResumed"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_RESUMED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainShutdown"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_SHUTDOWN"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainStarted"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_STARTED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainStopped"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_STOPPED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainSuspended"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_SUSPENDED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainUndefined"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_UNDEFINED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index e79fb5e..4b927a8 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -3,58 +3,108 @@ <node name="/org/libvirt/domain"> <interface name="org.libvirt.Domain"> - <property name="Name" type="s" access="read"/> - <property name="UUID" type="s" access="read"/> - <property name="Id" type="u" access="read"/> - <property name="OSType" type="s" access="read"/> - <property name="Active" type="b" access="read"/> - <property name="Persistent" type="b" access="read"/> - <property name="State" type="s" access="read"/> - <property name="Autostart" type="b" access="read"/> + <property name="Name" type="s" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetName"/> + </property> + <property name="UUID" type="s" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetUUID"/> + </property> + <property name="Id" type="u" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetID"/> + </property> + <property name="OSType" type="s" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetOSType"/> + </property> + <property name="Active" type="b" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainIsActive"/> + </property> + <property name="Persistent" type="b" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainIsPersistent"/> + </property> + <property name="State" type="s" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetState"/> + </property> + <property name="Autostart" type="b" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetAutostart"/> + </property> <method name="GetVcpus"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetVcpusFlags"/> <arg name="flags" type="u" direction="in"/> <arg name="vcpus" type="u" direction="out"/> </method> <method name="GetXMLDesc"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetXMLDesc"/> <arg name="flags" type="u" direction="in"/> <arg name="xml" type="s" direction="out"/> </method> <method name="GetStats"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainListGetStats"/> <arg name="stats" type="u" direction="in"/> <arg name="flags" type="u" direction="in"/> <arg name="records" type="a{sv}" direction="out"/> </method> <method name="Shutdown"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainShutdownFlags"/> <arg name="flags" type="u" direction="in"/> </method> <method name="Destroy"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDestroyFlags"/> <arg name="flags" type="u" direction="in"/> </method> <method name="Reboot"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainReboot"/> <arg name="flags" type="u" direction="in"/> </method> <method name="Reset"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainReset"/> <arg name="flags" type="u" direction="in"/> </method> <method name="Create"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCreateWithFlags"/> <arg name="flags" type="u" direction="in"/> </method> <method name="Undefine"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainUndefineFlags"/> <arg name="flags" type="u" direction="in"/> </method> <signal name="DeviceAdded"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_ID_DEVICE_ADDED"/> <arg name="device" type="s"/> </signal> <signal name="DeviceRemoved"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED"/> <arg name="device" type="s"/> </signal> <signal name="DiskChange"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_ID_DISK_CHANGE"/> <arg name="oldSrcPath" type="s"/> <arg name="newSrcPath" type="s"/> <arg name="device" type="s"/> <arg name="reason" type="s"/> </signal> <signal name="TrayChange"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_ID_TRAY_CHANGE"/> <arg name="device" type="s"/> <arg name="reason" type="s"/> </signal> -- 2.15.0
On Fri, Mar 23, 2018 at 05:58:04PM +0100, Katerina Koukiou wrote:
Since we don't follow the exact naming of libvirt API for libvirt-dbus, documentation should clarify which API call is used internally each time.
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> --- data/org.libvirt.Connect.xml | 24 ++++++++++++++++ data/org.libvirt.Domain.xml | 66 ++++++++++++++++++++++++++++++++++++++------ 2 files changed, 82 insertions(+), 8 deletions(-)
diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml index 787cd8d..9849abe 100644 --- a/data/org.libvirt.Connect.xml +++ b/data/org.libvirt.Connect.xml @@ -4,51 +4,75 @@ <node name="/org/libvirt/connect"> <interface name="org.libvirt.Connect"> <method name="ListDomains"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectListDomains"/>
The function should be virConnectListAllDomains. I'll fix that before pushing.
<arg name="flags" type="u" direction="in"/> <arg name="domains" type="ao" direction="out"/> </method> <method name="CreateXML"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainCreateXML"/> <arg name="xml" type="s" direction="in"/> <arg name="flags" type="u" direction="in"/> <arg name="domain" type="o" direction="out"/> </method> <method name="DefineXML"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainDefineXML"/> <arg name="xml" type="s" direction="in"/> <arg name="domain" type="o" direction="out"/> </method> <signal name="DomainCrashed"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_CRASHED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal>
I was thinking about these domain lifecycle signals and maybe we should rewrite it into one signal and as a doc string we should mention the event callback function, which describes the arguments as well <https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectDomainEventCallback>. And unrelated to this patch itself, I've just noticed, that the XML describes the first argument as reason, which is not correct, the code sets the first argument to the domain name. So I'm thinking about having the signal like this: <signal name="Domain"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virConnectDomainEventCallback"/> <arg name="event" type="s"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> I'll fix the links for the other events to point to the callback function and the domain lifecycle events can be fixed by followup patches.
<signal name="DomainDefined"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_DEFINED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainPMSuspended"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_SUSPENDED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainResumed"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_RESUMED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainShutdown"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_SHUTDOWN"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainStarted"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_STARTED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainStopped"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_STOPPED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainSuspended"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_SUSPENDED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> <signal name="DomainUndefined"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#VIR_DOMAIN_EVENT_UNDEFINED"/> <arg name="reason" type="s"/> <arg name="domain" type="o"/> </signal> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index e79fb5e..4b927a8 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -3,58 +3,108 @@
<node name="/org/libvirt/domain"> <interface name="org.libvirt.Domain"> - <property name="Name" type="s" access="read"/> - <property name="UUID" type="s" access="read"/> - <property name="Id" type="u" access="read"/> - <property name="OSType" type="s" access="read"/> - <property name="Active" type="b" access="read"/> - <property name="Persistent" type="b" access="read"/> - <property name="State" type="s" access="read"/> - <property name="Autostart" type="b" access="read"/> + <property name="Name" type="s" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetName"/> + </property> + <property name="UUID" type="s" access="read"> + <annotation name="org.gtk.GDBus.DocString" + value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetUUID"/>
This should be virDomainGetUUIDString, I'll fix that before pushing. Pavel
On Fri, Mar 23, 2018 at 05:57:58PM +0100, Katerina Koukiou wrote:
* Use API calls with flags parameter when possible * Since libvirt-dbus API doesn't follow strictly the naming of libvirt API there should be documentation regarding which libvirt API is used for each libvirt-dbus API. This patch introduces documentation annotations in D-Bus Interface XML files which will be used later to generate proper documentation.
Katerina Koukiou (6): virtDBusDomainGetVcpus: Should be implemented as method and not as property virtDBusDomainShutdown: Use virDomainShutdownFlags instead of virDomainShutdown virtDBusDomainCreate: Use virDomainCreateWithFlags instead of virDomainCreate virtDBusDomainUndefine: Use virDomainUndefineFlags instead of virDomainUndefine virtDBusDomainDestroy: Use virDomainDestroyFlags instead of virDomainDestroy Use Documentation Annotations in D-Bus Interface XML
Thanks for the patches, I'll fix the issues before pushing. Reviewed-by: Pavel Hrdina <phrdina@redhat.com>
participants (2)
-
Katerina Koukiou -
Pavel Hrdina