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

[Daniel P. Berrangé]

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