On Tue, Feb 19, 2013 at 14:26:37 +0000, Daniel P. Berrange wrote:
> On Mon, Feb 18, 2013 at 05:07:44PM +0100, Jiri Denemark wrote:
> > To avoid having to hold the qemu driver lock while iterating through
> > close callbacks and calling them. This fixes a real deadlock when a
> > domain which is being migrated from another host gets autodestoyed as a
> > result of broken connection to the other host.
>
> Since you're turning this into a full object, I think it would be
> nice to move it to a standalone file, so it can be reused by the
> LXC driver, which currently re-invents this same kind of code.
Good idea, could it be done by a follow-up patch? What do you think
would be the best name for such a shared object? I guess something like
virConnectCloseCallbacks could make sense...
> > -VIR_ONCE_GLOBAL_INIT(virQEMUConfig)
> > +static int
> > +virQEMUCloseCallbacksOnceInit(void)
> > +{
> > + virQEMUCloseCallbacksClass = virClassNew(virClassForObjectLockable(),
> > + "virQEMUCloseCallbacks",
> > + sizeof(virQEMUCloseCallbacks),
> > + virQEMUCloseCallbacksDispose);
> > + if (!virQEMUCloseCallbacksClass)
> > + return -1;
> > + return 0;
> > +}
> >
> > +VIR_ONCE_GLOBAL_INIT(virQEMUConfig)
> > +VIR_ONCE_GLOBAL_INIT(virQEMUCloseCallbacks)
>
> No need for two initializers, just make the virQEMUConfigOnceInit
> method create both classes.
OK, they will need to be separated again once this is decoupled from
qemu driver but that's fine.
> > -typedef virDomainObjPtr (*qemuDriverCloseCallback)(virQEMUDriverPtr driver,
> > - virDomainObjPtr vm,
> > - virConnectPtr conn);
> > -int qemuDriverCloseCallbackInit(virQEMUDriverPtr driver);
> > -void qemuDriverCloseCallbackShutdown(virQEMUDriverPtr driver);
> > -int qemuDriverCloseCallbackSet(virQEMUDriverPtr driver,
> > +typedef virDomainObjPtr (*virQEMUCloseCallback)(virQEMUDriverPtr driver,
> > + virDomainObjPtr vm,
> > + virConnectPtr conn);
> > +virQEMUCloseCallbacksPtr virQEMUCloseCallbacksNew(void);
> > +int virQEMUCloseCallbacksSet(virQEMUDriverPtr driver,
>
> I was rather expecting to see the virQEMUDriverPtr replacd
> with a virQEMUCloseCallbacksPtr in all the methods now it
> becomes a full object.
I guess I was just lazy and didn't want to change callers too much :-) I
would need to do that when moving the code out of qemu driver anyway,
though.
> ACK to the general approach, but I think there's a few tweaks needed
> still.
OK, v2 is coming soon.
Jirka
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list