Re: [libvirt] [PATCH 20/20] remote/rpc: Use virNetServerGetProgram() to determine the program

Use virNetServerGetProgram() to determine the virNetServerProgram instead of using hard coded global variables. This allows us to remove the global variables @remoteProgram and @qemuProgram as they're now no longer necessary. Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com&gt; Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; --- Note: I'm not 100% sure that there is no case where the lock for @client is already held by the thread which is calling virNetServerGetProgram and thus the lock order would be violated (the lock order has to be @server -> @client in the violating case it would be @client -> @server and therefore a deadlock might occur).

--- src/libvirt_remote.syms | 1 + src/remote/remote_daemon.c | 4 +-- src/remote/remote_daemon.h | 3 --- src/remote/remote_daemon_dispatch.c | 52 ++++++++++++++++++------------------- src/rpc/gendispatch.pl | 2 ++ src/rpc/virnetserver.c | 23 ++++++++++++++++ src/rpc/virnetserver.h | 2 ++ 7 files changed, 56 insertions(+), 31 deletions(-)

diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 97e22275b980..c31b16cd5909 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -120,6 +120,7 @@ virNetServerGetCurrentUnauthClients; virNetServerGetMaxClients; virNetServerGetMaxUnauthClients; virNetServerGetName; +virNetServerGetProgram; virNetServerGetThreadPoolParameters; virNetServerHasClients; virNetServerNew; diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index 078430f91c97..64f67ef2fefb 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -71,8 +71,6 @@ VIR_LOG_INIT("daemon.libvirtd"); #if WITH_SASL virNetSASLContextPtr saslCtxt = NULL; #endif -virNetServerProgramPtr remoteProgram = NULL; -virNetServerProgramPtr qemuProgram = NULL; volatile bool driversInitialized = false; @@ -1061,6 +1059,8 @@ int main(int argc, char **argv) { virNetServerPtr srv = NULL; virNetServerPtr srvAdm = NULL; virNetServerProgramPtr adminProgram = NULL; + virNetServerProgramPtr qemuProgram = NULL; + virNetServerProgramPtr remoteProgram = NULL; virNetServerProgramPtr lxcProgram = NULL; char *remote_config_file = NULL; int statuswrite = -1; diff --git a/src/remote/remote_daemon.h b/src/remote/remote_daemon.h index 4467f71da910..93f2e1f531b0 100644 --- a/src/remote/remote_daemon.h +++ b/src/remote/remote_daemon.h @@ -82,7 +82,4 @@ struct daemonClientPrivate { # if WITH_SASL extern virNetSASLContextPtr saslCtxt; # endif -extern virNetServerProgramPtr remoteProgram; -extern virNetServerProgramPtr qemuProgram; - #endif /* __REMOTE_DAEMON_H__ */ diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 95940ffdeefe..3dce6195230e 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3808,9 +3808,9 @@ remoteDispatchNodeDeviceGetParent(virNetServerPtr server ATTRIBUTE_UNUSED, } static int -remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr) { int rv = -1; @@ -3828,7 +3828,7 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server ATTRIBUTE_UNUS if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg));

/* eventID, callbackID, and legacy are not used */ callback->eventID = -1; callback->callbackID = -1; @@ -3912,7 +3912,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = VIR_DOMAIN_EVENT_ID_LIFECYCLE; callback->callbackID = -1; callback->legacy = true; @@ -4110,9 +4110,9 @@ remoteDispatchDomainGetState(virNetServerPtr server ATTRIBUTE_UNUSED, * VIR_DRV_SUPPORTS_FEATURE(VIR_DRV_FEATURE_REMOTE_EVENT_CALLBACK), * and must not mix the two styles. */ static int -remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_domain_event_register_any_args *args) { @@ -4149,7 +4149,7 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; callback->legacy = true; @@ -4185,9 +4185,9 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU static int -remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_domain_event_callback_register_any_args *args, remote_connect_domain_event_callback_register_any_ret *ret) @@ -4226,7 +4226,7 @@ remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server ATTRI if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -5352,7 +5352,7 @@ remoteDispatchDomainMigratePrepareTunnel3Params(virNetServerPtr server ATTRIBUTE goto cleanup; if (!(st = virStreamNew(priv->conn, VIR_STREAM_NONBLOCK)) || - !(stream = daemonCreateClientStream(client, st, remoteProgram, + !(stream = daemonCreateClientStream(client, st, virNetServerGetProgram(server, msg), &msg->header, false))) goto cleanup; @@ -5709,9 +5709,9 @@ static int remoteDispatchDomainCreateWithFiles(virNetServerPtr server ATTRIBUTE_ static int -remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_network_event_register_any_args *args, remote_connect_network_event_register_any_ret *ret) @@ -5750,7 +5750,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -5832,9 +5832,9 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_ } static int -remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_storage_pool_event_register_any_args *args, remote_connect_storage_pool_event_register_any_ret *ret) @@ -5873,7 +5873,7 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUT if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -5954,9 +5954,9 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr server ATTRIB } static int -remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_node_device_event_register_any_args *args, remote_connect_node_device_event_register_any_ret *ret) @@ -5995,7 +5995,7 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -6076,9 +6076,9 @@ remoteDispatchConnectNodeDeviceEventDeregisterAny(virNetServerPtr server ATTRIBU } static int -remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, +remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, remote_connect_secret_event_register_any_args *args, remote_connect_secret_event_register_any_ret *ret) @@ -6117,7 +6117,7 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(remoteProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -6198,9 +6198,9 @@ remoteDispatchConnectSecretEventDeregisterAny(virNetServerPtr server ATTRIBUTE_U } static int -qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED, +qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server, virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessagePtr msg, virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, qemu_connect_domain_monitor_event_register_args *args, qemu_connect_domain_monitor_event_register_ret *ret) @@ -6234,7 +6234,7 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_U if (VIR_ALLOC(callback) < 0) goto cleanup; callback->client = virObjectRef(client); - callback->program = virObjectRef(qemuProgram); + callback->program = virObjectRef(virNetServerGetProgram(server, msg)); callback->eventID = -1; callback->callbackID = -1; ref = callback; diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index fb15cc4849bc..725319bf8da4 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -990,6 +990,7 @@ elsif ($mode eq "server") { if ($call->{streamflag} ne "none") { print " virStreamPtr st = NULL;\n"; print " daemonClientStreamPtr stream = NULL;\n"; + print " virNetServerProgramPtr remoteProgram = NULL;\n"; if ($call->{sparseflag} ne "none") { print " const bool sparse = args->flags & $call->{sparseflag};\n" } else { @@ -1037,6 +1038,7 @@ elsif ($mode eq "server") { print " if (!(st = virStreamNew(priv->conn, VIR_STREAM_NONBLOCK)))\n"; print " goto cleanup;\n"; print "\n"; + print " remoteProgram = virNetServerGetProgram(server, msg);\n"; print " if (!(stream = daemonCreateClientStream(client, st, remoteProgram, &msg->header, sparse)))\n"; print " goto cleanup;\n"; print "\n"; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 4cd42ad7fd40..b1ff4a0cd751 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -205,6 +205,29 @@ virNetServerGetProgramLocked(virNetServerPtr srv, } +/** + * virNetServerGetProgram: + * @srv: server (must NOT be locked by the caller) + * @msg: message + * + * Searches @srv for the right program for a given message @msg. + * + * Returns a pointer to the server program or NULL if not found. + */ +virNetServerProgramPtr +virNetServerGetProgram(virNetServerPtr srv, + virNetMessagePtr msg) +{ + virNetServerProgramPtr ret = NULL; + + virObjectLock(srv); + ret = virNetServerGetProgramLocked(srv, msg); + virObjectUnlock(srv); + + return ret; +} + + static int virNetServerDispatchNewMessage(virNetServerClientPtr client, virNetMessagePtr msg, void *opaque) diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index a79c39fdb2e7..1867e46664ba 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -76,6 +76,8 @@ int virNetServerSetTLSContext(virNetServerPtr srv, virNetTLSContextPtr tls); # endif +virNetServerProgramPtr virNetServerGetProgram(virNetServerPtr srv, + virNetMessagePtr msg); int virNetServerAddClient(virNetServerPtr srv, virNetServerClientPtr client);