Re: [Qemu-devel] [PATCH 2/7] Replace VMSTOP macros with a proper QemuState type

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2011-08-04 11:55, Avi Kivity wrote:
> On 08/03/2011 06:17 PM, Luiz Capitulino wrote:
>> @@ -9,6 +9,20 @@
>>   #include "notify.h"
>>
>>   /* vl.c */
>> +
>> +typedef enum {
>> +    QSTATE_DEBUG,         /* qemu is running under gdb */
>> +    QSTATE_INTERROR,      /* paused due to an internal error */
>> +    QSTATE_IOERROR,       /* paused due to an I/O error */
>> +    QSTATE_PAUSED,        /* paused by the user (ie. the 'stop'
>> command) */
>> +    QSTATE_PREMIGRATE,    /* paused preparing to finish migrate */
>> +    QSTATE_RESTVM,        /* paused restoring the VM state */
>> +    QSTATE_RUNNING,       /* qemu is running */
>> +    QSTATE_SAVEVM,        /* paused saving VM state */
>> +    QSTATE_SHUTDOWN,      /* guest shut down and -no-shutdown is in
>> use */
>> +    QSTATE_WATCHDOG       /* watchdog fired and qemu is configured to
>> pause */
>> +} QemuState;
>> +
>>   extern const char *bios_name;
>>
> 
> Why "QemuState"?  In general, "qemu" can be inferred from the fact that
> we're in qemu.git.  Suggest "RunState".
> 
> Second, these states can coexist.  A user may pause the VM
> simultaneously with the watchdog firing or entering premigrate state. 
> In fact, with multiple monitors, each monitor can pause and resume the
> vm independently.
> 
> So I think we should keep a reference count instead of just a start/stop
> state.  Perhaps
> 
> vm_stop(QemuState s)
> {
>     ++stopcount[s];
> }
> 
> vm_is_stopped()
> {
>     for (s in states)
>           if (stopcount[s])
>               return true;
>     return false;
> }

I don't think this makes sense nor is user-friendly. If one command
channel suspends the machine, others have the chance to subscribe for
that event. Maintaining a suspension counter would mean you also need a
channel to query its value.

IMHO, there is also no use for defining stopped orthogonally to
premigrate and other states that imply that the machine is stopped.
Basically they mean "stopped for/because of X". We just need to avoid
that you can enter plain stopped state from them by issuing the
corresponding monitor command. The other way around might be possible,
though, if there are race windows.

Jan


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