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