On Wed, Jan 10, 2018 at 12:23:31PM -0500, John Ferlan wrote:
> From: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
