Re: [Qemu-devel] [PATCH 6/7] Monitor: Don't allow cont on bad VM state

[Jan Kiszka]

On 2011-08-03 19:32, Luiz Capitulino wrote:
> On Wed, 03 Aug 2011 17:32:03 +0200
> Jan Kiszka <address@hidden> wrote:
> 
>> On 2011-08-03 17:17, Luiz Capitulino wrote:
>>> We have two states where issuing cont before system_reset can be
>>> catastrophic: QSTATE_SHUTDOWN (when -no-shutdown is used) and
>>> QSTATE_INTERROR (which only happen with kvm).
>>>
>>> This commit fixes that by making system_reset mandatory before
>>> issuing cont in those states.
>>>
>>> Signed-off-by: Luiz Capitulino <address@hidden>
>>> ---
>>>  cpus.c    |    4 ++++
>>>  monitor.c |    8 ++++++++
>>>  qerror.c  |    4 ++++
>>>  qerror.h  |    3 +++
>>>  sysemu.h  |    2 +-
>>>  vl.c      |    1 +
>>>  6 files changed, 21 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/cpus.c b/cpus.c
>>> index 65ea503..a61e658 100644
>>> --- a/cpus.c
>>> +++ b/cpus.c
>>> @@ -125,6 +125,10 @@ static void do_vm_stop(QemuState state)
>>>          pause_all_vcpus();
>>>          qemu_state_set(state);
>>>          vm_state_notify(0, state);
>>> +        if (state == QSTATE_INTERROR || state == QSTATE_SHUTDOWN) {
>>> +            /* system_reset is required by 'cont' */
>>> +            system_reset_required = 1;
>>> +        }
>>>          qemu_aio_flush();
>>>          bdrv_flush_all();
>>>          monitor_protocol_event(QEVENT_STOP, NULL);
>>> diff --git a/monitor.c b/monitor.c
>>> index 3fa2cf7..f1cb5af 100644
>>> --- a/monitor.c
>>> +++ b/monitor.c
>>> @@ -1312,7 +1312,14 @@ static int do_cont(Monitor *mon, const QDict *qdict, 
>>> QObject **ret_data)
>>>      if (qemu_state_get() == QSTATE_INMIGRATE) {
>>>          qerror_report(QERR_MIGRATION_EXPECTED);
>>>          return -1;
>>> +    } else if (qemu_state_get() == QSTATE_INTERROR ||
>>> +               qemu_state_get() == QSTATE_SHUTDOWN) {
>>> +        if (system_reset_required) {
>>> +            qerror_report(QERR_RESET_REQUIRED);
>>> +            return -1;
>>> +        }
>>
>> Why not just enter a proper state, likely QSTATE_PAUSED, when resetting
>> over INTERROR or SHUTDOWN? Would save you system_reset_required and make
>> the state machine simpler.
> 
> Yes, seems to be a good idea.

Forgot to say that the rest looks OK to me.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux