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

[Pavel Hrdina]

On Wed, Nov 13, 2019 at 07:12:34PM +0100, Marc Hartmayer wrote:
> On Wed, Nov 13, 2019 at 09:52 AM +0100, Pavel Hrdina <phrdina at redhat.com> 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 at linux.ibm.com>
> 
> […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')?

Returning directly would not work properly as well, we call
virNetMessageSaveError() in case of error in the cleanup section.

Another label would work.

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

We don't have to hold the lock for virNetServerGetProgram(), it just
makes the function easier to follow as there will be only one cleanup
and I don't see any harm of holding the lock for that function call as
well if the function will later wait for the same lock.

Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191114/9dead71e/attachment-0001.sig>