On Wed, Mar 16, 2016 at 12:47:53PM +0100, Erik Skultety wrote:
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@xxxxxxxxxx> --- 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 :).
It stayed rotting in one of my branches for a while and when I needed to send it, I forgot that it's not fixed completely. Not even Jan found that out, so at least now I know somebody reads that ;) Thanks for spotting it.
+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.
Yes, that's another thing I'm keeping in my mind which I hope I don't forget. However, nobody knows if the same things as with the description is not going to happen. Thanks for the review.
Erik
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list