Re: [Qemu-devel] [PATCH 1/1] qmp: process system-reset event in paused state

[Peter Krempa]

On Wed, Dec 16, 2015 at 10:12:20 +0100, Paolo Bonzini wrote:
> 
> 
> On 16/12/2015 10:00, Denis V. Lunev wrote:
> > With pvpanic or HyperV panic devices could be moved into the paused state
> > with ' <on_crash>preserve</on_crash>'. In this state VM reacts only to
> > 'virsh destroy' or 'continue'.
> > 
> > 'virsh reset' command is usually used to force guest reset. The expectation
> > of the behavior of this command is that the guest will be force restarted.
> > This is not true at the moment.
> 
> Does "virsh reset" + "virsh continue" work, and if not why?

So .. it won't work:

    state = virDomainObjGetState(vm, NULL);
    if (state == VIR_DOMAIN_PMSUSPENDED) {
        virReportError(VIR_ERR_OPERATION_INVALID,
                       "%s", _("domain is pmsuspended"));
        goto endjob;
    } else if (state == VIR_DOMAIN_PAUSED) {
        if (qemuProcessStartCPUs(driver, vm, dom->conn,
                                 VIR_DOMAIN_RUNNING_UNPAUSED,
                                 QEMU_ASYNC_JOB_NONE) < 0) {
            if (virGetLastError() == NULL)
                virReportError(VIR_ERR_OPERATION_FAILED,
                               "%s", _("resume operation failed"));
            goto endjob;
        }
        event = virDomainEventLifecycleNewFromObj(vm,
                                         VIR_DOMAIN_EVENT_RESUMED,
                                         VIR_DOMAIN_EVENT_RESUMED_UNPAUSED);
    }


We check that the state is "paused" and continue the vCPUs only in that
case. The panic devices will move the VM to 'crashed' state.
The code that is issuing 'system_reset' does not modify the state
in any way.

> 
> > Thus it is quite natural to process 'virh reset' aka qmp_system_reset
> > this way, i.e. allow to reset the guest. This behavior is similar to
> > one observed with 'reset' button on real hardware :)
> 
> Paolo
> 
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > CC: Markus Armbruster <address@hidden>
> > CC: Dmitry Andreev <address@hidden>
> > ---
> >  qmp.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/qmp.c b/qmp.c
> > index 0a1fa19..df17a33 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -112,6 +112,10 @@ void qmp_stop(Error **errp)
> >  void qmp_system_reset(Error **errp)
> >  {
> >      qemu_system_reset_request();
> > +
> > +    if (!runstate_is_running()) {
> > +        vm_start();
> > +    }

I'd say NACK here. This will break the possibility to reset a system
while the vCPUs are paused. The problem should be fixed in libvirt.

> >  }
> >  
> >  void qmp_system_powerdown(Error **erp)
> > 
> 

Peter

signature.asc
Description: Digital signature