On Wed, 2016-11-23 at 11:00 -0500, Dawid Zamirski wrote:
> On Wed, 2016-11-23 at 08:55 -0500, John Ferlan wrote:
> > [...]
> > > +
> > > +static vboxDriverPtr
> > > +vboxGetDriverConnection(void)
> > > +{
> > > + virMutexLock(&vbox_driver_lock);
> > > +
> > > + if (vbox_driver) {
> > > + virObjectRef(vbox_driver);
> > > + } else {
> > > + vbox_driver = vboxDriverObjNew();
> > > +
> > > + if (!vbox_driver) {
> > > + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > > + _("Failed to create vbox driver
> > > object."));
> > > + return NULL;
> > > + }
> > > + }
> > > +
> > > + if (vboxSdkInitialize() < 0 || vboxExtractVersion() < 0) {
> >
> > In this path should vboxSdkUninitialize be called (since it
> > wouldn't
> > be
> > called in the destroy path)?
>
> If vboxSdkUnintialize fails, VBoxSVC is not started so it does not
> need
> to be unintialized - which is in line with the sample code included
> SDK
> where it returns with EXIT_FAILUE in main if pfnClientInifialize
> fails.
> However, if vboxExtractVersion fails (unlikely) we might want to call
> gVBoxAPI.UPFN.Unintialize(vbox_driver) directly (not via
> vboxSdkUninitialize as it checks connectionCount > 0 which on failure
> would be 0). I've just tested both cases with
> gVBoxAPI.UPFN.Unintialize
> in the failure path, and calling it does not do any harm in both
> cases,
> so I guess it would be a good idea to put it there.
>
Actually, I was wrong in that it's safe to call
gVBoxAPI.UPFN.Unintialize when vboxSdkInitialize fails - I got segfault
when attempting to connect twice in VBOX_RELEASE(data->vboxObject).
It's safe to do this though:
if (vboxSdkInitialize() < 0) {
virObjectUnref(vbox_driver);
virMutexUnlock(&vbox_driver_lock);
return NULL;
}
if (vboxExtractVersion() < 0) {
gVBoxAPI.UPFN.Uninitialize(vbox_driver);
virObjectUnref(vbox_driver);
virMutexUnlock(&vbox_driver_lock);
return NULL;
}
i.e do not uninitalize on initialize failure, but do unintialize on
vboxExractVersion failure.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
