[libvirt] [dbus PATCH 0/4] various fixes
Pavel Hrdina (4): configure: Bump required libvirt version Revert "Implement BridgeName property for Network interface." Annotate properties for which we will not emit changed signal Annotate properties that will never change during the object lifecycle configure.ac | 2 +- data/org.libvirt.Connect.xml | 5 +++++ data/org.libvirt.Domain.xml | 10 ++++++++++ data/org.libvirt.Network.xml | 9 +++++---- src/network.c | 23 ----------------------- tests/test_network.py | 1 - 6 files changed, 21 insertions(+), 29 deletions(-) -- 2.14.3
We will implement all libvirt APIs up to libvirt 3.0.0 version for now. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 6f166f2..cc8947c 100644 --- a/configure.ac +++ b/configure.ac @@ -12,7 +12,7 @@ AC_USE_SYSTEM_EXTENSIONS AM_SILENT_RULES([yes]) GLIB2_REQUIRED=2.44.0 -LIBVIRT_REQUIRED=2.1.0 +LIBVIRT_REQUIRED=3.0.0 LIBVIRT_GLIB_REQUIRED=0.0.7 AC_SUBST([GLIB2_REQUIRED]) dnl used in the .spec file AC_SUBST([LIBVIRT_REQUIRED]) dnl used in the .spec file -- 2.14.3
On Thu, 2018-05-03 at 14:46 +0200, Pavel Hrdina wrote:
We will implement all libvirt APIs up to libvirt 3.0.0 version for now.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Katerina Koukiou <kkoukiou@redhat.com>
This reverts commit 5b28d8f778d6c5b7ebd64909882b19d70cdc098f. This API is broken by design since you cannot specify whether it returns bridge name from persistent or active XML definition. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- data/org.libvirt.Network.xml | 4 ---- src/network.c | 23 ----------------------- tests/test_network.py | 1 - 3 files changed, 28 deletions(-) diff --git a/data/org.libvirt.Network.xml b/data/org.libvirt.Network.xml index cf05062..81bf081 100644 --- a/data/org.libvirt.Network.xml +++ b/data/org.libvirt.Network.xml @@ -12,10 +12,6 @@ value="See https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetAutostart and https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkSetAutostart"/> </property> - <property name="BridgeName" type="s" access="read"> - <annotation name="org.gtk.GDBus.DocString" - value="See https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetBridgeName"/> - </property> <property name="Name" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetName"/> diff --git a/src/network.c b/src/network.c index 4d00dfe..cf846d7 100644 --- a/src/network.c +++ b/src/network.c @@ -108,28 +108,6 @@ virtDBusNetworkGetAutostart(const gchar *objectPath, *value = g_variant_new("b", !!autostart); } -static void -virtDBusNetworkGetBridgeName(const gchar *objectPath, - gpointer userData, - GVariant **value, - GError **error) -{ - virtDBusConnect *connect = userData; - g_autoptr(virNetwork) network = NULL; - g_autofree gchar *bridge = NULL; - - network = virtDBusNetworkGetVirNetwork(connect, objectPath, error); - if (!network) - return; - - bridge = virNetworkGetBridgeName(network); - - if (!bridge) - return virtDBusUtilSetLastVirtError(error); - - *value = g_variant_new("s", bridge); -} - static void virtDBusNetworkGetName(const gchar *objectPath, gpointer userData, @@ -405,7 +383,6 @@ virtDBusNetworkUpdate(GVariant *inArgs, static virtDBusGDBusPropertyTable virtDBusNetworkPropertyTable[] = { { "Active", virtDBusNetworkGetActive, NULL }, { "Autostart", virtDBusNetworkGetAutostart, virtDBusNetworkSetAutostart }, - { "BridgeName", virtDBusNetworkGetBridgeName, NULL }, { "Name", virtDBusNetworkGetName, NULL }, { "Persistent", virtDBusNetworkGetPersistent, NULL }, { "UUID", virtDBusNetworkGetUUID, NULL }, diff --git a/tests/test_network.py b/tests/test_network.py index 2c1bd21..b91bc93 100755 --- a/tests/test_network.py +++ b/tests/test_network.py @@ -20,7 +20,6 @@ class TestNetwork(libvirttest.BaseTestClass): props = obj.GetAll('org.libvirt.Network', dbus_interface=dbus.PROPERTIES_IFACE) assert isinstance(props['Active'], dbus.Boolean) assert isinstance(props['Autostart'], dbus.Boolean) - assert isinstance(props['BridgeName'], dbus.String) assert isinstance(props['Name'], dbus.String) assert isinstance(props['Persistent'], dbus.Boolean) assert isinstance(props['UUID'], dbus.String) -- 2.14.3
On Thu, 2018-05-03 at 14:46 +0200, Pavel Hrdina wrote:
This reverts commit 5b28d8f778d6c5b7ebd64909882b19d70cdc098f.
This API is broken by design since you cannot specify whether it returns bridge name from persistent or active XML definition.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- data/org.libvirt.Network.xml | 4 ---- src/network.c | 23 ----------------------- tests/test_network.py | 1 - 3 files changed, 28 deletions(-)
Reviewed-by: Katerina Koukiou <kkoukiou@redhat.com>
For some of these properties there is no libvirt event to detect the change and for properties where we could somehow detect the change let's annotate them as well. We could change the properties to methods but with the annotation we can keep them as properties in order to allow to get them by single D-Bus call. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- data/org.libvirt.Connect.xml | 3 +++ data/org.libvirt.Domain.xml | 9 +++++++++ data/org.libvirt.Network.xml | 3 +++ 3 files changed, 15 insertions(+) diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml index ee7bfdc..3bddb89 100644 --- a/data/org.libvirt.Connect.xml +++ b/data/org.libvirt.Connect.xml @@ -11,10 +11,12 @@ <property name="Hostname" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetHostname"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="LibVersion" type="t" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetLibVersion"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="Secure" type="b" access="read"> <annotation name="org.gtk.GDBus.DocString" @@ -24,6 +26,7 @@ <property name="Version" type="t" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetVersion"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <method name="BaselineCPU"> <annotation name="org.gtk.GDBus.DocString" diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index db43b1c..fcd3221 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -6,39 +6,48 @@ <property name="Active" type="b" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainIsActive"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="Autostart" type="b" access="readwrite"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetAutostart and https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainSetAutostart"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </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"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="Name" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetName"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </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"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </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"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="SchedulerType" type="(si)" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetSchedulerType"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </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"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="Updated" type="b" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainIsUpdated"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="UUID" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" diff --git a/data/org.libvirt.Network.xml b/data/org.libvirt.Network.xml index 81bf081..e31417c 100644 --- a/data/org.libvirt.Network.xml +++ b/data/org.libvirt.Network.xml @@ -6,11 +6,13 @@ <property name="Active" type="b" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkIsActive"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="Autostart" type="b" access="readwrite"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetAutostart and https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkSetAutostart"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="Name" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" @@ -19,6 +21,7 @@ <property name="Persistent" type="b" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkIsPersistent"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="false"/> </property> <property name="UUID" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" -- 2.14.3
On Thu, 2018-05-03 at 14:46 +0200, Pavel Hrdina wrote:
For some of these properties there is no libvirt event to detect the change and for properties where we could somehow detect the change let's annotate them as well.
We could change the properties to methods but with the annotation we can keep them as properties in order to allow to get them by single D-Bus call.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- data/org.libvirt.Connect.xml | 3 +++ data/org.libvirt.Domain.xml | 9 +++++++++ data/org.libvirt.Network.xml | 3 +++ 3 files changed, 15 insertions(+)
Maybe it would be nicer to set `false` as the default annotation for EmitsChangedSignal in the enclosing interface element and explicitly mark the rest. But I am ok with this as well. Reviewed-by: Katerina Koukiou <kkoukiou@redhat.com>
On Thu, May 03, 2018 at 03:21:01PM +0200, Katerina Koukiou wrote:
On Thu, 2018-05-03 at 14:46 +0200, Pavel Hrdina wrote:
For some of these properties there is no libvirt event to detect the change and for properties where we could somehow detect the change let's annotate them as well.
We could change the properties to methods but with the annotation we can keep them as properties in order to allow to get them by single D-Bus call.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- data/org.libvirt.Connect.xml | 3 +++ data/org.libvirt.Domain.xml | 9 +++++++++ data/org.libvirt.Network.xml | 3 +++ 3 files changed, 15 insertions(+)
Maybe it would be nicer to set `false` as the default annotation for EmitsChangedSignal in the enclosing interface element and explicitly mark the rest. But I am ok with this as well.
Good point, I missed that in D-Bus specification that the annotation can be for the whole interface. I'll sent v2. Thanks, Pavel
These can be annotated as 'const' properties because they will never change. Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- data/org.libvirt.Connect.xml | 2 ++ data/org.libvirt.Domain.xml | 1 + data/org.libvirt.Network.xml | 2 ++ 3 files changed, 5 insertions(+) diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml index 3bddb89..f4d17e9 100644 --- a/data/org.libvirt.Connect.xml +++ b/data/org.libvirt.Connect.xml @@ -7,6 +7,7 @@ <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectIsEncrypted Note that monitoring of traffic on the D-Bus message bus is out of the scope of this property"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="const"/> </property> <property name="Hostname" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" @@ -22,6 +23,7 @@ <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectIsSecure Note that monitoring of traffic on the D-Bus message bus is out of the scope of this property"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="const"/> </property> <property name="Version" type="t" access="read"> <annotation name="org.gtk.GDBus.DocString" diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml index fcd3221..224f5aa 100644 --- a/data/org.libvirt.Domain.xml +++ b/data/org.libvirt.Domain.xml @@ -52,6 +52,7 @@ <property name="UUID" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainGetUUIDString"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="const"/> </property> <method name="AbortJob"> <annotation name="org.gtk.GDBus.DocString" diff --git a/data/org.libvirt.Network.xml b/data/org.libvirt.Network.xml index e31417c..d939f4a 100644 --- a/data/org.libvirt.Network.xml +++ b/data/org.libvirt.Network.xml @@ -17,6 +17,7 @@ <property name="Name" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetName"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="const"/> </property> <property name="Persistent" type="b" access="read"> <annotation name="org.gtk.GDBus.DocString" @@ -26,6 +27,7 @@ <property name="UUID" type="s" access="read"> <annotation name="org.gtk.GDBus.DocString" value="See https://libvirt.org/html/libvirt-libvirt-network.html#virNetworkGetUUIDString"/> + <annotation name="org.freedesktop.DBus.Property.EmitsChangedSignal" value="const"/> </property> <method name="Create"> <annotation name="org.gtk.GDBus.DocString" -- 2.14.3
On Thu, 2018-05-03 at 14:46 +0200, Pavel Hrdina wrote:
These can be annotated as 'const' properties because they will never change.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> ---
data/org.libvirt.Connect.xml | 2 ++ data/org.libvirt.Domain.xml | 1 + data/org.libvirt.Network.xml | 2 ++ 3 files changed, 5 insertions(+)
Reviewed-by: Katerina Koukiou <kkoukiou@redhat.com>
participants (2)
-
Katerina Koukiou -
Pavel Hrdina