
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@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