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

Re: [libvirt] [PATCH 18/21] admin: Add virAdmConnectLookupServer



On 10/03/16 05:54, Martin Kletzander wrote:
> It does not have a suffix ByName because there are no other means of
> looking up the server and since the name is known, this should be the
> preferred one.
> 
> Signed-off-by: Martin Kletzander <mkletzan redhat com>
> ---
>  daemon/admin_server.c           | 10 ++++++++++
>  daemon/admin_server.h           |  5 +++++
>  include/libvirt/libvirt-admin.h |  4 ++++
>  src/admin/admin_protocol.x      | 16 +++++++++++++++-
>  src/admin_protocol-structs      |  8 ++++++++
>  src/libvirt-admin.c             | 36 ++++++++++++++++++++++++++++++++++++
>  src/libvirt_admin_private.syms  |  2 ++
>  src/libvirt_admin_public.syms   |  1 +
>  8 files changed, 81 insertions(+), 1 deletion(-)
> 
> ...
> +
> +/**
> + * virAdmConnectLookupServer:
> + * @conn: daemon connection reference
> + * @name: name of the server too lookup
> + * @flags: unused, must be 0
> + *
> + * Collect list of all servers provided by daemon the client is connected to.
> + *
> + * Returns the number of servers available on daemon side or -1 in case of a
> + * failure, setting @servers to NULL. There is a guaranteed extra element set
> + * to NULL in the @servers list returned to make the iteration easier, excluding
> + * this extra element from the final count.
> + * Caller is responsible to call virAdmServerFree() on each list element,
> + * followed by freeing @servers.
> + */

^^ This isn't quite describing what the function does, is it? Power of
copy-paste-edit I guess :).

> +virAdmServerPtr
> +virAdmConnectLookupServer(virAdmConnectPtr conn,
> +                          const char *name,
> +                          unsigned int flags)
> +{
> +    virAdmServerPtr ret = NULL;
> +
> +    VIR_DEBUG("conn=%p, name=%s, flags=%x", conn, NULLSTR(name), flags);
> +    virResetLastError();
> +
> +    virCheckAdmConnectGoto(conn, cleanup);
> +    virCheckNonNullArgGoto(name, cleanup);
> +    virCheckFlagsGoto(0, cleanup);
> +
> +    ret = remoteAdminConnectLookupServer(conn, name, flags);
> + cleanup:
> +    if (!ret)
> +        virDispatchError(NULL);
> +    return ret;
> +}
> diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms
> index f22137bcc6af..268f1e6f2fdd 100644
> --- a/src/libvirt_admin_private.syms
> +++ b/src/libvirt_admin_private.syms
> @@ -9,6 +9,8 @@
>  xdr_admin_connect_get_lib_version_ret;
>  xdr_admin_connect_list_servers_args;
>  xdr_admin_connect_list_servers_ret;
> +xdr_admin_connect_lookup_server_args;
> +xdr_admin_connect_lookup_server_ret;
>  xdr_admin_connect_open_args;
> 
>  # datatypes.h
> diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms
> index 52ff2dc1b411..58d085e1f118 100644
> --- a/src/libvirt_admin_public.syms
> +++ b/src/libvirt_admin_public.syms
> @@ -24,4 +24,5 @@ LIBVIRT_ADMIN_1.3.0 {
>          virAdmConnectListServers;
>          virAdmServerGetName;
>          virAdmServerFree;
> +        virAdmConnectLookupServer;
>  };

Just a reminder, not that we should do something about it right now, but
let's not forget about the fact, that all the symbols are introduced one
after another within different releases. It's been already 2 releases,
since we pushed first admin-related code. So, before we enable admin,
the library version will have to be changed from 1.3.0 to the
appropriate version of the exact release in which we finally enable it.

> 

ACK with that commentary mentioned above changed.

Erik


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