On Wed, Jan 10, 2018 at 12:23:31PM -0500, John Ferlan wrote:
From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Shutdown function should help API calls to finish when event loop is not running anymore. For this reason let's close agent and qemu monitors. These function will unblock API calls wating for response from qemu process or qemu agent.
Closing agent monitor and setting priv->agent to NULL when waiting for response is normal usecase (handling EOF from agent is handled the same way for example).
However we can not do the same for qemu monitor. This monitor is normally closed and unrefed during qemuProcessStop under destroy job so other threads do not deal with priv->mon. But if we take extra reference to monitor we are good. This can lead to double close but this function looks like to be idempotent.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a203c9297..1de236cb5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1093,6 +1093,44 @@ qemuStateStop(void) return ret; }
+ +static int +qemuDomainDisconnect(virDomainObjPtr vm, void *opaque ATTRIBUTE_UNUSED) +{ + + qemuDomainObjPrivatePtr priv; + + virObjectLock(vm); + priv = vm->privateData; + + if (priv->mon) { + /* Take extra reference to monitor so it won't be disposed + * by qemuMonitorClose last unref. */ + virObjectRef(priv->mon); + qemuMonitorClose(priv->mon); + } + + if (priv->agent) { + /* Other threads are ready for priv->agent to became NULL meanwhile */ + qemuAgentClose(priv->agent); + priv->agent = NULL; + } + + virObjectUnlock(vm); + return 0; +} + + +static void +qemuStateShutdown(void) +{ + if (!qemu_driver) + return; + + virDomainObjListForEach(qemu_driver->domains, qemuDomainDisconnect, NULL); +} + + /** * qemuStateCleanup: * @@ -21357,6 +21395,7 @@ static virStateDriver qemuStateDriver = { .stateCleanup = qemuStateCleanup, .stateReload = qemuStateReload, .stateStop = qemuStateStop, + .stateShutdown = qemuStateShutdown,
I'm curious why we need this separate .stateShutdown hook. It feels like this code could just be placed at the start of qemuStateStop and achieve the same result. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|