On Fri, Dec 15, 2017 at 01:37 PM +0100, "Daniel P. Berrange" <[email protected]> wrote: > On Tue, Dec 12, 2017 at 12:36:27PM +0100, Marc Hartmayer wrote: >> Additionally, use a whitelist model to decide whether authentication >> is needed or not. > > Is this actually fixing any real problem, if so please document what > the problem is.
It 'tells' the developer what we’re doing here. At first glance, it wasn’t obvious to me how the authentication process works in libvirt. This change doesn’t hurt someone, but at least it should be easier for other developer to understand. Additionally, the default return value is changing to True (=> client is not authenticated). So if something may change at the authentication process (for example the value for the enums - I know it’s unlikely to happen here…) we’re sure that the user isn’t authenticated by accident. > > AFAICT, this is mostly just a case of painting the bikeshed a different > colour, as both old & new code seem to have the same result in all cases ? > >> >> Signed-off-by: Marc Hartmayer <[email protected]> >> Reviewed-by: Boris Fiuczynski <[email protected]> >> Reviewed-by: Bjoern Walk <[email protected]> >> Reviewed-by: Stefan Zimmermann <[email protected]> >> --- >> src/rpc/virnetserverclient.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c >> index b454a3ff6992..0ee299e2d6ec 100644 >> --- a/src/rpc/virnetserverclient.c >> +++ b/src/rpc/virnetserverclient.c >> @@ -1512,10 +1512,10 @@ int >> virNetServerClientSendMessage(virNetServerClientPtr client, >> >> bool virNetServerClientNeedAuth(virNetServerClientPtr client) >> { >> - bool need = false; >> + bool need = true; >> virObjectLock(client); >> - if (client->auth) >> - need = true; >> + if (client->auth == VIR_NET_SERVER_SERVICE_AUTH_NONE) >> + need = false; >> virObjectUnlock(client); >> return need; >> } >> -- >> 2.13.4 >> >> -- >> libvir-list mailing list >> [email protected] >> https://www.redhat.com/mailman/listinfo/libvir-list > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| > -- Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
