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

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(a)redhat.com&gt; > > --- > > 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.