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

On Wed, Nov 13, 2019 at 09:52 AM +0100, Pavel Hrdina <phrdina(a)redhat.com&gt; wrote: > On Fri, Nov 01, 2019 at 06:35:48PM +0100, Marc Hartmayer wrote: >> 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.ibm.com&gt; […snip…] >> virNetMessageErrorPtr rerr) >> { >> int rv = -1; >> @@ -4180,6 +4180,12 @@ remoteDispatchConnectRegisterCloseCallback(virNetServerPtr server G_GNUC_UNUSED, >> struct daemonClientPrivate *priv = >> virNetServerClientGetPrivateData(client); >> virConnectPtr conn = remoteGetHypervisorConn(client); >> + virNetServerProgramPtr program; >> + >> + if (!(program = virNetServerGetProgram(server, msg))) { >> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("no matching program found")); >> + goto cleanup; >> + } > > This doesn't look right. If the function fails we will jump to cleanup > where we will try to unlock &priv->lock. This has to happen after we > acquire that lock. > > Pavel Yep, will fix that as well. Shall I directly return in the error case or jump to another label (e.g. 'cleanup_unlock')?

Or do see any reason why we should hold the priv->lock during the virNetServerGetProgram call?