Re: [libvirt] [PATCH 1/2] qemu: Turn closeCallbacks into virObjectLockable

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.

> -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.

> -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.

ACK to the general approach, but I think there's a few tweaks needed still.