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