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

[Ján Tomko]

On 01/12/2015 08:02 PM, John Ferlan wrote:
> 
> 
> 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?

Well, we'd overwrite 'end of file from monitor' by 'domain is no longer
running' and personally I don't think it matters.

Jan

> 
> ACK
> 
> John


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150113/64c66179/attachment-0001.sig>