Re: [libvirt] [PATCH 2/2] admin: Remove flags checking from virAdmConnectOpen public API

On 05.05.2016 13:49, Erik Skultety wrote: > Unlike the previous commit, we do actually support one client-side only flag > VIR_CONNECT_NO_ALIASES, so besides removing the check for flags this flag > has to be masked out before sending a message to the daemon, otherwise it > would trigger an error when checking flags on the daemon side. > > Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; > --- > src/admin/admin_remote.c | 2 +- > src/libvirt-admin.c | 4 ++-- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c > index 40fcddb..180dd3b 100644 > --- a/src/admin/admin_remote.c > +++ b/src/admin/admin_remote.c > @@ -162,7 +162,7 @@ remoteAdminConnectOpen(virAdmConnectPtr conn, unsigned int flags) > > virObjectLock(priv); > > - args.flags = flags; > + args.flags = flags ^ VIR_CONNECT_NO_ALIASES; As Peter already pointed out, this should be flags & ~VIR_CONNECT_NO_ALIASES; > > if (virNetClientRegisterAsyncIO(priv->client) < 0) { > VIR_DEBUG("Failed to add event watch, disabling events and support for" > diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c > index d7a66ff..91f40c8 100644 > --- a/src/libvirt-admin.c > +++ b/src/libvirt-admin.c > @@ -185,7 +185,8 @@ virAdmGetDefaultURI(virConfPtr conf) > /** > * virAdmConnectOpen: > * @name: uri of the daemon to connect to, NULL for default > - * @flags: extra flags; not used yet, so callers should always pass 0 > + * @flags: extra flags; currently the only supported client-side flag is > + * VIR_CONNECT_NO_ALIASES > * I'd rather have here the regular description ("bitwise or of virConnectFlags ...") and document that the only supported flag so far is _NO_ALIASES. Thing is, I'd rather avoid enumerating all supported flags here as the list might be extensive over time. > * Opens connection to admin interface of the daemon. > * > @@ -204,7 +205,6 @@ virAdmConnectOpen(const char *name, unsigned int flags) > > VIR_DEBUG("flags=%x", flags); > virResetLastError(); > - virCheckFlagsGoto(VIR_CONNECT_NO_ALIASES, error); > > if (!(conn = virAdmConnectNew())) > goto error; > ACK with those nits fixed. Michal