Re: [libvirt] [PATCH 01/11] Introduce qemuDomainObjExitMonitorAlive

On 12/17/2014 01:21 PM, Daniel P. Berrange wrote: > On Wed, Dec 17, 2014 at 01:18:41PM +0100, Michal Privoznik wrote: >> On 16.12.2014 17:41, 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. >>> >>> Introduce a helper function for the callers that want to report >>> an error in this case. >>> --- >>> src/qemu/qemu_domain.c | 17 +++++++++++++++++ >>> src/qemu/qemu_domain.h | 3 +++ >>> 2 files changed, 20 insertions(+) >>> >>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c >>> index 8dd427a..d074429 100644 >>> --- a/src/qemu/qemu_domain.c >>> +++ b/src/qemu/qemu_domain.c >>> @@ -1606,6 +1606,23 @@ void qemuDomainObjExitMonitor(virQEMUDriverPtr driver, >>> qemuDomainObjExitMonitorInternal(driver, obj); >>> } >>> >>> +/* obj must NOT be locked before calling >>> + * >>> + * Same as qemuDomainObjExitMonitor, but reports an error and >>> + * returns -1 if the domain is no longer alive after exiting the monitor >> >> Can you state explicitly that this function needs to be called whenever the >> domain object or vm->def, or privdata (any runtime info held within vm in >> general) is used after ExitMonitor()? > Right, I can add this to qemu/THREADS.txt as well. > Are there any cases where that is not true ? eg could we just avoid > introducing this new function, by making the existing funtion check > liveliness every time. That would avoid the class of bugs you outline > above entirely. I was worried about backports, but new code checking the return value would fail to compile with the void function in older libvirt.

There are functions that only call EndJob and return after exiting the monitor. But if the domain crashed before the API even returned, can we really claim success?