On 01/07/2015 10:42 AM, Ján Tomko wrote:
> The domain might disappear during the time in monitor when
> the virDomainObjPtr is unlocked, so the caller needs to check
> if it's still alive.
>
> Since most of the callers are going to need it, put the
> check inside qemuDomainObjExitMonitor and return -1 if
> the domain died in the meantime.
> ---
> src/qemu/THREADS.txt | 5 +++++
> src/qemu/qemu_domain.c | 16 ++++++++++++++--
> src/qemu/qemu_domain.h | 4 ++--
> 3 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt
> index add2a35..dfa372b 100644
> --- a/src/qemu/THREADS.txt
> +++ b/src/qemu/THREADS.txt
> @@ -160,6 +160,11 @@ To acquire the QEMU monitor lock
> - Acquires the virDomainObjPtr lock
>
> These functions must not be used by an asynchronous job.
> + Note that the virDomainObj is unlocked during the time in
> + monitor and it can be changed, e.g. if QEMU dies, qemuProcessStop
> + may free the live domain definition and put the persistent
> + definition back in vm->def. The callers should check the return
> + value of ExitMonitor to see if the domain is still alive.
>
>
> To acquire the QEMU monitor lock as part of an asynchronous job
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 3d4023c..c942b02 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -1586,11 +1586,23 @@ void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver,
> /* obj must NOT be locked before calling
> *
> * Should be paired with an earlier qemuDomainObjEnterMonitor() call
> + *
> + * Returns -1 if the domain is no longer alive after exiting the monitor.
> + * In that case, the caller should be careful when using obj's data,
> + * e.g. the live definition in vm->def has been freed by qemuProcessStop
> + * and replaced by the persistent definition, so pointers stolen
> + * from the live definition could no longer be valid.
> */
> -void qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
> - virDomainObjPtr obj)
> +int qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
> + virDomainObjPtr obj)
> {
> qemuDomainObjExitMonitorInternal(driver, obj);
> + if (!virDomainObjIsActive(obj)) {
> + virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> + _("domain is no longer running"));
> + return -1;
> + }
> + return 0;
Do we have to worry about caller paths perhaps using virReportError and
this overwriting the other message for the last message? Does it really
matter?
ACK
John
> }
>
> /*
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 6b52f03..fd91d83 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -245,8 +245,8 @@ void qemuDomainObjReleaseAsyncJob(virDomainObjPtr obj);
> void qemuDomainObjEnterMonitor(virQEMUDriverPtr driver,
> virDomainObjPtr obj)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> -void qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
> - virDomainObjPtr obj)
> +int qemuDomainObjExitMonitor(virQEMUDriverPtr driver,
> + virDomainObjPtr obj)
> ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
> int qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr driver,
> virDomainObjPtr obj,
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list