1. Patchew
  2. Libvirt
  3. [libvirt] [dbus PATCH 0/8] Properties/Methods for Connect Interface
  4. View patch

[libvirt] [dbus PATCH 1/8] Implement Capabilities property for connect Interface

Katerina Koukiou posted 8 patches 7 years, 1 month ago
There is a newer version of this series
[libvirt] [dbus PATCH 1/8] Implement Capabilities property for connect Interface
Posted by Katerina Koukiou 7 years, 1 month ago 
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
---
 data/org.libvirt.Connect.xml |  4 ++++
 src/connect.c                | 20 ++++++++++++++++++++
 test/test_connect.py         |  1 +
 3 files changed, 25 insertions(+)

diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
index e36b7f6..caf0a47 100644
--- a/data/org.libvirt.Connect.xml
+++ b/data/org.libvirt.Connect.xml
@@ -3,6 +3,10 @@
 
 <node name="/org/libvirt/connect">
   <interface name="org.libvirt.Connect">
+    <property name="Capabilities" type="s" access="read">
+      <annotation name="org.gtk.GDBus.DocString"
+        value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetCapabilities"/>
+    </property>
     <property name="Version" type="t" access="read">
       <annotation name="org.gtk.GDBus.DocString"
         value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetVersion"/>
diff --git a/src/connect.c b/src/connect.c
index ce9055a..af61cc8 100644
--- a/src/connect.c
+++ b/src/connect.c
@@ -91,6 +91,25 @@ virtDBusConnectOpen(virtDBusConnect *connect,
     return TRUE;
 }
 
+static void
+virtDBusConnectGetCapabilities(const gchar *objectPath G_GNUC_UNUSED,
+                               gpointer userData,
+                               GVariant **value,
+                               GError **error)
+{
+    virtDBusConnect *connect = userData;
+    g_autofree gchar *capabilities = NULL;
+
+    if (!virtDBusConnectOpen(connect, error))
+        return;
+
+    capabilities = virConnectGetCapabilities(connect->connection);
+    if (!capabilities)
+        return virtDBusUtilSetLastVirtError(error);
+
+    *value = g_variant_new("s", capabilities);
+}
+
 static void
 virtDBusConnectGetVersion(const gchar *objectPath G_GNUC_UNUSED,
                           gpointer userData,
@@ -443,6 +462,7 @@ virtDBusNetworkLookupByUUID(GVariant *inArgs,
 }
 
 static virtDBusGDBusPropertyTable virtDBusConnectPropertyTable[] = {
+    { "Capabilities", virtDBusConnectGetCapabilities, NULL },
     { "Version", virtDBusConnectGetVersion, NULL },
     { 0 }
 };
diff --git a/test/test_connect.py b/test/test_connect.py
index c411eeb..d7c1e20 100755
--- a/test/test_connect.py
+++ b/test/test_connect.py
@@ -81,6 +81,7 @@ class TestConnect(libvirttest.BaseTestClass):
         assert original_path == path
 
     @pytest.mark.parametrize("property_name,expected_type", [
+        ("Capabilities", dbus.String),
         ("Version", dbus.UInt64),
     ])
     def test_connect_properties_return_type(self, property_name, expected_type):
-- 
2.15.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 1/8] Implement Capabilities property for connect Interface
Posted by Pavel Hrdina 7 years, 1 month ago 
On Mon, Apr 09, 2018 at 01:47:32PM +0200, Katerina Koukiou wrote:
> Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
> ---
>  data/org.libvirt.Connect.xml |  4 ++++
>  src/connect.c                | 20 ++++++++++++++++++++
>  test/test_connect.py         |  1 +
>  3 files changed, 25 insertions(+)

Adding Dan to CC because I'm not that familiar with D-Bus conventions.
This API can be exported as property, however it returns rather large
string and not a simple value so I'm not sure whether method would be
better in this case.

Pavel

> 
> diff --git a/data/org.libvirt.Connect.xml b/data/org.libvirt.Connect.xml
> index e36b7f6..caf0a47 100644
> --- a/data/org.libvirt.Connect.xml
> +++ b/data/org.libvirt.Connect.xml
> @@ -3,6 +3,10 @@
>  
>  <node name="/org/libvirt/connect">
>    <interface name="org.libvirt.Connect">
> +    <property name="Capabilities" type="s" access="read">
> +      <annotation name="org.gtk.GDBus.DocString"
> +        value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetCapabilities"/>
> +    </property>
>      <property name="Version" type="t" access="read">
>        <annotation name="org.gtk.GDBus.DocString"
>          value="See https://libvirt.org/html/libvirt-libvirt-host.html#virConnectGetVersion"/>
> diff --git a/src/connect.c b/src/connect.c
> index ce9055a..af61cc8 100644
> --- a/src/connect.c
> +++ b/src/connect.c
> @@ -91,6 +91,25 @@ virtDBusConnectOpen(virtDBusConnect *connect,
>      return TRUE;
>  }
>  
> +static void
> +virtDBusConnectGetCapabilities(const gchar *objectPath G_GNUC_UNUSED,
> +                               gpointer userData,
> +                               GVariant **value,
> +                               GError **error)
> +{
> +    virtDBusConnect *connect = userData;
> +    g_autofree gchar *capabilities = NULL;
> +
> +    if (!virtDBusConnectOpen(connect, error))
> +        return;
> +
> +    capabilities = virConnectGetCapabilities(connect->connection);
> +    if (!capabilities)
> +        return virtDBusUtilSetLastVirtError(error);
> +
> +    *value = g_variant_new("s", capabilities);
> +}
> +
>  static void
>  virtDBusConnectGetVersion(const gchar *objectPath G_GNUC_UNUSED,
>                            gpointer userData,
> @@ -443,6 +462,7 @@ virtDBusNetworkLookupByUUID(GVariant *inArgs,
>  }
>  
>  static virtDBusGDBusPropertyTable virtDBusConnectPropertyTable[] = {
> +    { "Capabilities", virtDBusConnectGetCapabilities, NULL },
>      { "Version", virtDBusConnectGetVersion, NULL },
>      { 0 }
>  };
> diff --git a/test/test_connect.py b/test/test_connect.py
> index c411eeb..d7c1e20 100755
> --- a/test/test_connect.py
> +++ b/test/test_connect.py
> @@ -81,6 +81,7 @@ class TestConnect(libvirttest.BaseTestClass):
>          assert original_path == path
>  
>      @pytest.mark.parametrize("property_name,expected_type", [
> +        ("Capabilities", dbus.String),
>          ("Version", dbus.UInt64),
>      ])
>      def test_connect_properties_return_type(self, property_name, expected_type):
> -- 
> 2.15.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 1/8] Implement Capabilities property for connect Interface
Posted by Daniel P. Berrangé 7 years, 1 month ago 
On Mon, Apr 09, 2018 at 02:25:04PM +0200, Pavel Hrdina wrote:
> On Mon, Apr 09, 2018 at 01:47:32PM +0200, Katerina Koukiou wrote:
> > Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
> > ---
> >  data/org.libvirt.Connect.xml |  4 ++++
> >  src/connect.c                | 20 ++++++++++++++++++++
> >  test/test_connect.py         |  1 +
> >  3 files changed, 25 insertions(+)
> 
> Adding Dan to CC because I'm not that familiar with D-Bus conventions.
> This API can be exported as property, however it returns rather large
> string and not a simple value so I'm not sure whether method would be
> better in this case.

The concept of properties is essentially a convenience to make live easier
for mapping into language bindings / higher level object systems.

At the protocol level, the properties are handling via a standard interface
with a handful of methods

https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-properties

So whether you have a "Capabilities" property, or a "GetCapabilities"
method, you'll still have a method call on the wire protocol, with the
full string transmitted. There is also a signal that servers should
emit when a property value changes so that clients know that if they
have cached the property value, they should refetch it.

So potential issues are

 - If we don't emit PropertyChange signals, clients may cache out
   of date capabilities
 - If clients pre-fetch properties there'll be a small perf hit
 - If we have lots of properties, then the overall size of all
   properties combined might cause problems for GetAll

The first item is the one that would concern me in this instance.
The libvirt capabilities data can change between calls, and libvirt
has no way to notify you of such changes. Thus IMHO it is better
exposed as a dynamic method call than a property.  Leave properties
for libvirt things that are esentially static data, unless we're
able to provide PropertyChange signals.



Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [dbus PATCH 1/8] Implement Capabilities property for connect Interface
Posted by Katerina Koukiou 7 years, 1 month ago 
On Mon, 2018-04-09 at 13:45 +0100, Daniel P. Berrangé wrote:
> On Mon, Apr 09, 2018 at 02:25:04PM +0200, Pavel Hrdina wrote:
> > On Mon, Apr 09, 2018 at 01:47:32PM +0200, Katerina Koukiou wrote:
> > > Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
> > > ---
> > >  data/org.libvirt.Connect.xml |  4 ++++
> > >  src/connect.c                | 20 ++++++++++++++++++++
> > >  test/test_connect.py         |  1 +
> > >  3 files changed, 25 insertions(+)
> > 
> > Adding Dan to CC because I'm not that familiar with D-Bus
> > conventions.
> > This API can be exported as property, however it returns rather
> > large
> > string and not a simple value so I'm not sure whether method would
> > be
> > better in this case.
> 
> The concept of properties is essentially a convenience to make live
> easier
> for mapping into language bindings / higher level object systems.
> 
> At the protocol level, the properties are handling via a standard
> interface
> with a handful of methods
> 
> https://dbus.freedesktop.org/doc/dbus-specification.html#standard-int
> erfaces-properties
> 
> So whether you have a "Capabilities" property, or a "GetCapabilities"
> method, you'll still have a method call on the wire protocol, with
> the
> full string transmitted. There is also a signal that servers should
> emit when a property value changes so that clients know that if they
> have cached the property value, they should refetch it.
> 
> So potential issues are
> 
>  - If we don't emit PropertyChange signals, clients may cache out
>    of date capabilities
>  - If clients pre-fetch properties there'll be a small perf hit
>  - If we have lots of properties, then the overall size of all
>    properties combined might cause problems for GetAll
> 
> The first item is the one that would concern me in this instance.
> The libvirt capabilities data can change between calls, and libvirt
> has no way to notify you of such changes. Thus IMHO it is better
> exposed as a dynamic method call than a property.  Leave properties
> for libvirt things that are esentially static data, unless we're
> able to provide PropertyChange signals.
> 

Thanks for the detailed explanation. I will adjust it and repost.

> 
> 
> Regards,
> Daniel


Regards,
Katerina

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.