[libvirt] [dbus PATCH 02/10] GetVcpus API method: Renamed to GetVcpusFlags

[Katerina Koukiou]

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 at 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 at 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 :|