[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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 redhat com>
> ---
>  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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]