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

[Martin Kletzander]

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 at 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 :).
>

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160316/2af416f6/attachment-0001.sig>