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

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(a)redhat.com&gt; >> --- >> 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.