Re: [libvirt] [PATCH 10/14] rpc: Introduce virNetServerSetClientAuthenticated

On 12/12/2017 06:36 AM, Marc Hartmayer wrote: > Introduce a function which marks the client as authenticated and also > it tracks on the server that the authentication for this client has > been completed. Afterwords it will check for the limits of the server. > > After using this new function the function > virNetServerTrackCompletedAuth was superfluous and is therefore > removed. In addition, it is not very common that a > '{{function}}' (virNetServerTrackCompletedAuth) does more than just > the locking compared to > '{{function}}Locked' (virNetServerTrackCompletedAuthLocked). > > virNetServerTrackPendingAuth was already superfluous and therefore > it's also removed. > So essentially you're combining virNetServerClientSetAuth and virNetServerTrackCompletedAuth into one new function and needed to rename the virNetServerClientSetAuth to the Locked variety.

> Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt; > Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; > Reviewed-by: Stefan Zimmermann <stzi(a)linux.vnet.ibm.com&gt; > --- > daemon/remote.c | 9 +++------ > src/libvirt_remote.syms | 5 ++--- > src/rpc/virnetserver.c | 40 ++++++++++++++++++++++------------------ > src/rpc/virnetserver.h | 3 +-- > src/rpc/virnetserverclient.c | 5 ++--- > src/rpc/virnetserverclient.h | 2 +- > 6 files changed, 31 insertions(+), 33 deletions(-) > Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt;

John one nit below... [...] > diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c > index d03bd3e91905..72105cd9318f 100644 > --- a/src/rpc/virnetserver.c > +++ b/src/rpc/virnetserver.c > @@ -737,6 +737,28 @@ int virNetServerSetTLSContext(virNetServerPtr srv, > #endif > > > +/** > + * virNetServerSetClientAuthenticated: > + * @srv: server must be unlocked > + * @client: client must be unlocked > + * > + * Mark @client as authenticated and tracks on @srv that the > + * authentication of this @client has been completed. Also it checks > + * the limits of @srv. > + */ > +void > +virNetServerSetClientAuthenticated(virNetServerPtr srv, virNetServerClientPtr client) One line for each argument... > +{ > + virObjectLock(srv); > + virObjectLock(client); > + virNetServerClientSetAuthLocked(client, VIR_NET_SERVER_SERVICE_AUTH_NONE); > + virNetServerTrackCompletedAuthLocked(srv); > + virNetServerCheckLimits(srv); > + virObjectUnlock(client); > + virObjectUnlock(srv); > +} > + > + [...]