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

["Denis V. Lunev" <den@openvz.org>]

On 12/16/2015 12:50 PM, Peter Krempa wrote:
> 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 <den@openvz.org>
>>> CC: Markus Armbruster <armbru@redhat.com>
>>> CC: Dmitry Andreev <dandreev@virtuozzo.com>
>>> ---
>>>   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.

I do not get your explanation, sorry. If vCPUs are paused
original code just sets flags and do nothing else, i.e.
reset in this state will never happens until external kick.

Anyway, I will be fine with any solution here :) This is a
matter of consideration. QEMU kludge is just a way shorter.

Den