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@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
>
> […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
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
