Re: [libvirt] [PATCHv2 01/14] Check for domain liveness in qemuDomainObjExitMonitor

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