[libvirt] [PATCH 3/5] qemu: Pass running reason to RESUME event handler

[John Ferlan]

On 9/21/18 8:12 AM, Jiri Denemark wrote:
> On Wed, Sep 19, 2018 at 11:12:26 -0400, John Ferlan wrote:
>>
>>
>> On 09/12/2018 08:55 AM, Jiri Denemark wrote:
>>> Whenever we get the RESUME event from QEMU, we change the state of the
>>> affected domain to VIR_DOMAIN_RUNNING with VIR_DOMAIN_RUNNING_UNPAUSED
>>> reason. This is fine if the domain is resumed unexpectedly, but when we
>>> sent "cont" to QEMU we usually have a better reason for the state
>>> change. The better reason is used in qemuProcessStartCPUs which also
>>> sets the domain state to running if qemuMonitorStartCPUs reports
>>> success. Thus we may end up with two state updates in a row, but the
>>> final reason is correct.
>>>
>>> This patch is a preparation for dropping the state change done in
>>> qemuMonitorStartCPUs for which we need to pass the actual running reason
>>> to the RESUME event handler and use it there instead of
>>> VIR_DOMAIN_RUNNING_UNPAUSED.
>>>
>>> Signed-off-by: Jiri Denemark <jdenemar at redhat.com>
>>> ---
>>>  src/qemu/qemu_domain.h  |  4 ++++
>>>  src/qemu/qemu_process.c | 23 +++++++++++++++++------
>>>  2 files changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
>>> index 914c9a6a8d..3f3f7ccf18 100644
>>> --- a/src/qemu/qemu_domain.h
>>> +++ b/src/qemu/qemu_domain.h
>>> @@ -366,6 +366,10 @@ struct _qemuDomainObjPrivate {
>>>  
>>>      /* counter for generating node names for qemu disks */
>>>      unsigned long long nodenameindex;
>>> +
>>> +    /* qemuProcessStartCPUs stores the reason for starting vCPUs here for the
>>> +     * RESUME event handler to use it */
>>> +    virDomainRunningReason runningReason;
>>
>> So what happens in the libvirtd restart case/condition?  This isn't
>> Format/Parse'd so it's lost or essentially set to RUNNING_UNKNOWN.
> 
> Right, when libvirtd is restarted just after sending "cont", I don't
> think we can expect to see the "RESUME" event in the new process. Most
> likely the previous libvirtd process already got the event and just
> didn't have a chance to process it. Thus just updating
> qemuDomainObjPrivateXML{Parse|Format} to store this in the status XML
> wouldn't help. We could add a code which would look at the restored
> runningReason when seeing vCPUs are running, but status XML says the
> domain is paused. But it would be in a separate patch anyway.
> 

OK - fair enough. I see something adding to ObjPrivate without the
Format/Parse and I think we should add it there.

However, in this case it's tricky because of the event driven mechanics
of change - especially when libvirtd isn't running and the event occurs.
Perhaps just note in qemu_domain.h why the Format/Parse of the value
isn't being done.

The R-By can still apply without the Format/Parse then.

John