Same for internal virtDBusDomainGetVcpus:
Renamed to virtDBusDomainGetVcpusFlags
Following naming from libvirt API.
Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
---
data/org.libvirt.Domain.xml | 2 +-
src/domain.c | 17 ++++++++---------
test/test_domain.py | 2 +-
3 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
index 1ecf826..46cc8a7 100644
--- a/data/org.libvirt.Domain.xml
+++ b/data/org.libvirt.Domain.xml
@@ -11,7 +11,7 @@
<property name="Persistent" type="b" access="read"/>
<property name="State" type="s" access="read"/>
<property name="Autostart" type="b" access="read"/>
- <method name="GetVcpus">
+ <method name="GetVcpusFlags">
<arg name="flags" type="u" direction="in"/>
<arg name="vcpus" type="u" direction="out"/>
</method>
diff --git a/src/domain.c b/src/domain.c
index 09b3440..333b5e8 100644
--- a/src/domain.c
+++ b/src/domain.c
@@ -219,14 +219,13 @@ virtDBusDomainGetAutostart(const gchar *objectPath,
}
static void
-virtDBusDomainGetVcpus(GVariant *inArgs,
- GUnixFDList *inFDs G_GNUC_UNUSED,
- const gchar *objectPath,
- gpointer userData,
- GVariant **outArgs,
- GUnixFDList **outFDs G_GNUC_UNUSED,
- GError **error)
-
+virtDBusDomainGetVcpusFlags(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;
@@ -451,7 +450,7 @@ static virtDBusGDBusPropertyTable virtDBusDomainPropertyTable[] = {
};
static virtDBusGDBusMethodTable virtDBusDomainMethodTable[] = {
- { "GetVcpus", virtDBusDomainGetVcpus },
+ { "GetVcpusFlags", virtDBusDomainGetVcpusFlags },
{ "GetXMLDesc", virtDBusDomainGetXMLDesc },
{ "GetStats", virtDBusDomainGetStats },
{ "Shutdown", virtDBusDomainShutdown },
diff --git a/test/test_domain.py b/test/test_domain.py
index 22039dc..c7f9ca2 100755
--- a/test/test_domain.py
+++ b/test/test_domain.py
@@ -28,7 +28,7 @@ class TestDomain(libvirttest.BaseTestClass):
xml = domain.GetXMLDesc(0)
assert isinstance(xml, dbus.String)
- vcpus = domain.GetVcpus(0)
+ vcpus = domain.GetVcpusFlags(0)
assert isinstance(vcpus, dbus.UInt32)
domain.Reboot(0)
--
2.15.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Mar 23, 2018 at 03:16:59PM +0100, Katerina Koukiou wrote:
> Same for internal virtDBusDomainGetVcpus:
> Renamed to virtDBusDomainGetVcpusFlags
>
> Following naming from libvirt API.
>
> Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
> ---
> data/org.libvirt.Domain.xml | 2 +-
> src/domain.c | 17 ++++++++---------
> test/test_domain.py | 2 +-
> 3 files changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> index 1ecf826..46cc8a7 100644
> --- a/data/org.libvirt.Domain.xml
> +++ b/data/org.libvirt.Domain.xml
> @@ -11,7 +11,7 @@
> <property name="Persistent" type="b" access="read"/>
> <property name="State" type="s" access="read"/>
> <property name="Autostart" type="b" access="read"/>
> - <method name="GetVcpus">
> + <method name="GetVcpusFlags">
We only added Flags as a suffix in our C apis because we have no other
way to fix it without breaking ABI compat.
In some language bindings, however, we didn't preserve that naming,
instead just adding 'flags' as an optional parameter.
DBus doesn't have optional params, but since this is a green-field API,
we don't have a backcompat problem to worry about.
IOW, I suggest *not* adding "Flags" as a suffix to any of the DBus
method names, even if you ultimately call a libvirt API named
with a "Flags" suffix.
> <arg name="flags" type="u" direction="in"/>
> <arg name="vcpus" type="u" direction="out"/>
> </method>
> diff --git a/src/domain.c b/src/domain.c
> index 09b3440..333b5e8 100644
> --- a/src/domain.c
> +++ b/src/domain.c
> @@ -219,14 +219,13 @@ virtDBusDomainGetAutostart(const gchar *objectPath,
> }
>
> static void
> -virtDBusDomainGetVcpus(GVariant *inArgs,
> - GUnixFDList *inFDs G_GNUC_UNUSED,
> - const gchar *objectPath,
> - gpointer userData,
> - GVariant **outArgs,
> - GUnixFDList **outFDs G_GNUC_UNUSED,
> - GError **error)
> -
> +virtDBusDomainGetVcpusFlags(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;
> @@ -451,7 +450,7 @@ static virtDBusGDBusPropertyTable virtDBusDomainPropertyTable[] = {
> };
>
> static virtDBusGDBusMethodTable virtDBusDomainMethodTable[] = {
> - { "GetVcpus", virtDBusDomainGetVcpus },
> + { "GetVcpusFlags", virtDBusDomainGetVcpusFlags },
> { "GetXMLDesc", virtDBusDomainGetXMLDesc },
> { "GetStats", virtDBusDomainGetStats },
> { "Shutdown", virtDBusDomainShutdown },
> diff --git a/test/test_domain.py b/test/test_domain.py
> index 22039dc..c7f9ca2 100755
> --- a/test/test_domain.py
> +++ b/test/test_domain.py
> @@ -28,7 +28,7 @@ class TestDomain(libvirttest.BaseTestClass):
>
> xml = domain.GetXMLDesc(0)
> assert isinstance(xml, dbus.String)
> - vcpus = domain.GetVcpus(0)
> + vcpus = domain.GetVcpusFlags(0)
> assert isinstance(vcpus, dbus.UInt32)
>
> domain.Reboot(0)
> --
> 2.15.0
>
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
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
On Fri, Mar 23, 2018 at 02:25:25PM +0000, Daniel P. Berrangé wrote: > On Fri, Mar 23, 2018 at 03:16:59PM +0100, Katerina Koukiou wrote: > > Same for internal virtDBusDomainGetVcpus: > > Renamed to virtDBusDomainGetVcpusFlags > > > > Following naming from libvirt API. > > > > Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com> > > --- > > data/org.libvirt.Domain.xml | 2 +- > > src/domain.c | 17 ++++++++--------- > > test/test_domain.py | 2 +- > > 3 files changed, 10 insertions(+), 11 deletions(-) > > > > diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml > > index 1ecf826..46cc8a7 100644 > > --- a/data/org.libvirt.Domain.xml > > +++ b/data/org.libvirt.Domain.xml > > @@ -11,7 +11,7 @@ > > <property name="Persistent" type="b" access="read"/> > > <property name="State" type="s" access="read"/> > > <property name="Autostart" type="b" access="read"/> > > - <method name="GetVcpus"> > > + <method name="GetVcpusFlags"> > > We only added Flags as a suffix in our C apis because we have no other > way to fix it without breaking ABI compat. > > In some language bindings, however, we didn't preserve that naming, > instead just adding 'flags' as an optional parameter. > > DBus doesn't have optional params, but since this is a green-field API, > we don't have a backcompat problem to worry about. > > IOW, I suggest *not* adding "Flags" as a suffix to any of the DBus > method names, even if you ultimately call a libvirt API named > with a "Flags" suffix. I was thinking about not following the names exactly but on the other hand it may leads into a confusion especially when we have the non-flags version of the same API. I personally don't like the API names and I would gladly remove the suffix from the D-Bus API names, but it may lead into a confusion about which libvirt API is used under the hood. Sure, the API takes flags so it will be probably the flags version, but we as libvirt developers know this fact, on the other hand users might not realize that. If we decide not to follow the libvirt API names, we should probably somehow document which libvirt API is used. Pavel -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Fri, Mar 23, 2018 at 03:43:45PM +0100, Pavel Hrdina wrote:
> On Fri, Mar 23, 2018 at 02:25:25PM +0000, Daniel P. Berrangé wrote:
> > On Fri, Mar 23, 2018 at 03:16:59PM +0100, Katerina Koukiou wrote:
> > > Same for internal virtDBusDomainGetVcpus:
> > > Renamed to virtDBusDomainGetVcpusFlags
> > >
> > > Following naming from libvirt API.
> > >
> > > Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
> > > ---
> > > data/org.libvirt.Domain.xml | 2 +-
> > > src/domain.c | 17 ++++++++---------
> > > test/test_domain.py | 2 +-
> > > 3 files changed, 10 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
> > > index 1ecf826..46cc8a7 100644
> > > --- a/data/org.libvirt.Domain.xml
> > > +++ b/data/org.libvirt.Domain.xml
> > > @@ -11,7 +11,7 @@
> > > <property name="Persistent" type="b" access="read"/>
> > > <property name="State" type="s" access="read"/>
> > > <property name="Autostart" type="b" access="read"/>
> > > - <method name="GetVcpus">
> > > + <method name="GetVcpusFlags">
> >
> > We only added Flags as a suffix in our C apis because we have no other
> > way to fix it without breaking ABI compat.
> >
> > In some language bindings, however, we didn't preserve that naming,
> > instead just adding 'flags' as an optional parameter.
> >
> > DBus doesn't have optional params, but since this is a green-field API,
> > we don't have a backcompat problem to worry about.
> >
> > IOW, I suggest *not* adding "Flags" as a suffix to any of the DBus
> > method names, even if you ultimately call a libvirt API named
> > with a "Flags" suffix.
>
> I was thinking about not following the names exactly but on the other
> hand it may leads into a confusion especially when we have the non-flags
> version of the same API.
>
> I personally don't like the API names and I would gladly remove the
> suffix from the D-Bus API names, but it may lead into a confusion about
> which libvirt API is used under the hood. Sure, the API takes flags
> so it will be probably the flags version, but we as libvirt developers
> know this fact, on the other hand users might not realize that.
>
> If we decide not to follow the libvirt API names, we should probably
> somehow document which libvirt API is used.
In the Go binding I did this in the embedded API docs via link
to the C API docs
// See also https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainAttachDevice
func (d *Domain) AttachDevice(xml string) error {
cXml := C.CString(xml)
defer C.free(unsafe.Pointer(cXml))
result := C.virDomainAttachDevice(d.ptr, cXml)
if result == -1 {
return GetLastError()
}
return nil
}
IIRC, the dbus introspection XML has something for docs comments where you
could do the same.
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
Right, as seen here [1], "documentation can also be added as
annotation elements in the XML". I 'll repost, thanks.
[1] https://dbus.freedesktop.org/doc/dbus-api-design.html#documentation
On Fri, Mar 23, 2018 at 3:51 PM, Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Fri, Mar 23, 2018 at 03:43:45PM +0100, Pavel Hrdina wrote:
>> On Fri, Mar 23, 2018 at 02:25:25PM +0000, Daniel P. Berrangé wrote:
>> > On Fri, Mar 23, 2018 at 03:16:59PM +0100, Katerina Koukiou wrote:
>> > > Same for internal virtDBusDomainGetVcpus:
>> > > Renamed to virtDBusDomainGetVcpusFlags
>> > >
>> > > Following naming from libvirt API.
>> > >
>> > > Signed-off-by: Katerina Koukiou <kkoukiou@redhat.com>
>> > > ---
>> > > data/org.libvirt.Domain.xml | 2 +-
>> > > src/domain.c | 17 ++++++++---------
>> > > test/test_domain.py | 2 +-
>> > > 3 files changed, 10 insertions(+), 11 deletions(-)
>> > >
>> > > diff --git a/data/org.libvirt.Domain.xml b/data/org.libvirt.Domain.xml
>> > > index 1ecf826..46cc8a7 100644
>> > > --- a/data/org.libvirt.Domain.xml
>> > > +++ b/data/org.libvirt.Domain.xml
>> > > @@ -11,7 +11,7 @@
>> > > <property name="Persistent" type="b" access="read"/>
>> > > <property name="State" type="s" access="read"/>
>> > > <property name="Autostart" type="b" access="read"/>
>> > > - <method name="GetVcpus">
>> > > + <method name="GetVcpusFlags">
>> >
>> > We only added Flags as a suffix in our C apis because we have no other
>> > way to fix it without breaking ABI compat.
>> >
>> > In some language bindings, however, we didn't preserve that naming,
>> > instead just adding 'flags' as an optional parameter.
>> >
>> > DBus doesn't have optional params, but since this is a green-field API,
>> > we don't have a backcompat problem to worry about.
>> >
>> > IOW, I suggest *not* adding "Flags" as a suffix to any of the DBus
>> > method names, even if you ultimately call a libvirt API named
>> > with a "Flags" suffix.
>>
>> I was thinking about not following the names exactly but on the other
>> hand it may leads into a confusion especially when we have the non-flags
>> version of the same API.
>>
>> I personally don't like the API names and I would gladly remove the
>> suffix from the D-Bus API names, but it may lead into a confusion about
>> which libvirt API is used under the hood. Sure, the API takes flags
>> so it will be probably the flags version, but we as libvirt developers
>> know this fact, on the other hand users might not realize that.
>>
>> If we decide not to follow the libvirt API names, we should probably
>> somehow document which libvirt API is used.
>
> In the Go binding I did this in the embedded API docs via link
> to the C API docs
>
>
> // See also https://libvirt.org/html/libvirt-libvirt-domain.html#virDomainAttachDevice
> func (d *Domain) AttachDevice(xml string) error {
> cXml := C.CString(xml)
> defer C.free(unsafe.Pointer(cXml))
> result := C.virDomainAttachDevice(d.ptr, cXml)
> if result == -1 {
> return GetLastError()
> }
> return nil
> }
>
> IIRC, the dbus introspection XML has something for docs comments where you
> could do the same.
>
>
> 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
© 2016 - 2025 Red Hat, Inc.