Re: [Qemu-devel] [PATCH v2 3/8] RunState: added two new flags for bitmap dump and migration process

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Sanidhya Kashyap (sanidhya.iiith@gmail.com) wrote:
> I have added two new flags - RUN_STATE_MIGRATE and RUN_STATE_DUMP_BITMAP.
> These both flags behave same as RUN_STATE_RUNNING flag. The purpose of
> introducing these flags is to avoid running both migration and dump bitmap
> process simultaneously.
> 
> I haven't added many transitions to the RUN_STATE_DUMP_BITMAP. I will try
> to include the transitions on the basis of discussions.
> 
> On the other hand, I have tried to add the transitions that might occur during
> the migration process. There is a possibility that some transitions can be
> redundant (as pointed by Chen, this is not my patch problem,  but I have tried
> to cover what I thought is necessary).

I find runstate to be very subtle; there are probably other things that
would need changing; e.g. I see the USB code has:
   if (state == RUN_STATE_RUNNING) {
and
   if (!runstate_check(RUN_STATE_RUNNING)) {

(it should probably be using runstate_is_running() )
Also virt-test tends to see the state (I don't know if libvirt etc do?)
so they may get confused.

Dave


> 
> Signed-off-by: Sanidhya Kashyap <sanidhya.iiith@gmail.com>
> ---
>  qapi-schema.json |  7 ++++++-
>  vl.c             | 29 ++++++++++++++++++++++++++++-
>  2 files changed, 34 insertions(+), 2 deletions(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 17e5147..2918fc4 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -234,12 +234,17 @@
>  # @watchdog: the watchdog action is configured to pause and has been triggered
>  #
>  # @guest-panicked: guest has been panicked as a result of guest OS panic
> +#
> +# @migrate: migration process is being executed
> +#
> +# @dump-bitmap: dump the writable working set of the guest
> +#
>  ##
>  { 'enum': 'RunState',
>    'data': [ 'debug', 'inmigrate', 'internal-error', 'io-error', 'paused',
>              'postmigrate', 'prelaunch', 'finish-migrate', 'restore-vm',
>              'running', 'save-vm', 'shutdown', 'suspended', 'watchdog',
> -            'guest-panicked' ] }
> +            'guest-panicked', 'migrate', 'dump-bitmap' ] }
>  
>  ##
>  # @SnapshotInfo
> diff --git a/vl.c b/vl.c
> index 709d8cd..a2ffd66 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -576,31 +576,39 @@ static const RunStateTransition runstate_transitions_def[] = {
>      /*     from      ->     to      */
>      { RUN_STATE_DEBUG, RUN_STATE_RUNNING },
>      { RUN_STATE_DEBUG, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_DEBUG, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_INMIGRATE, RUN_STATE_RUNNING },
>      { RUN_STATE_INMIGRATE, RUN_STATE_PAUSED },
>  
>      { RUN_STATE_INTERNAL_ERROR, RUN_STATE_PAUSED },
>      { RUN_STATE_INTERNAL_ERROR, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_INTERNAL_ERROR, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_IO_ERROR, RUN_STATE_RUNNING },
>      { RUN_STATE_IO_ERROR, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_IO_ERROR, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_PAUSED, RUN_STATE_RUNNING },
>      { RUN_STATE_PAUSED, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_PAUSED, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_POSTMIGRATE, RUN_STATE_RUNNING },
>      { RUN_STATE_POSTMIGRATE, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_POSTMIGRATE, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_PRELAUNCH, RUN_STATE_RUNNING },
>      { RUN_STATE_PRELAUNCH, RUN_STATE_FINISH_MIGRATE },
>      { RUN_STATE_PRELAUNCH, RUN_STATE_INMIGRATE },
> +    { RUN_STATE_PRELAUNCH, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_FINISH_MIGRATE, RUN_STATE_RUNNING },
>      { RUN_STATE_FINISH_MIGRATE, RUN_STATE_POSTMIGRATE },
>  
>      { RUN_STATE_RESTORE_VM, RUN_STATE_RUNNING },
>  
> +    { RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING},
> +
>      { RUN_STATE_RUNNING, RUN_STATE_DEBUG },
>      { RUN_STATE_RUNNING, RUN_STATE_INTERNAL_ERROR },
>      { RUN_STATE_RUNNING, RUN_STATE_IO_ERROR },
> @@ -611,6 +619,8 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RUNNING, RUN_STATE_SHUTDOWN },
>      { RUN_STATE_RUNNING, RUN_STATE_WATCHDOG },
>      { RUN_STATE_RUNNING, RUN_STATE_GUEST_PANICKED },
> +    { RUN_STATE_RUNNING, RUN_STATE_DUMP_BITMAP },
> +    { RUN_STATE_RUNNING, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_SAVE_VM, RUN_STATE_RUNNING },
>  
> @@ -621,12 +631,27 @@ static const RunStateTransition runstate_transitions_def[] = {
>      { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED },
>      { RUN_STATE_SUSPENDED, RUN_STATE_RUNNING },
>      { RUN_STATE_SUSPENDED, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_SUSPENDED, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_WATCHDOG, RUN_STATE_RUNNING },
>      { RUN_STATE_WATCHDOG, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_WATCHDOG, RUN_STATE_MIGRATE },
>  
>      { RUN_STATE_GUEST_PANICKED, RUN_STATE_RUNNING },
>      { RUN_STATE_GUEST_PANICKED, RUN_STATE_FINISH_MIGRATE },
> +    { RUN_STATE_GUEST_PANICKED, RUN_STATE_MIGRATE },
> +
> +    { RUN_STATE_DUMP_BITMAP, RUN_STATE_RUNNING },
> +
> +    { RUN_STATE_MIGRATE, RUN_STATE_POSTMIGRATE },
> +    { RUN_STATE_MIGRATE, RUN_STATE_PAUSED },
> +    { RUN_STATE_MIGRATE, RUN_STATE_SHUTDOWN },
> +    { RUN_STATE_MIGRATE, RUN_STATE_GUEST_PANICKED },
> +    { RUN_STATE_MIGRATE, RUN_STATE_DEBUG },
> +    { RUN_STATE_MIGRATE, RUN_STATE_RUNNING },
> +    { RUN_STATE_MIGRATE, RUN_STATE_INTERNAL_ERROR },
> +    { RUN_STATE_MIGRATE, RUN_STATE_IO_ERROR },
> +    { RUN_STATE_MIGRATE, RUN_STATE_WATCHDOG },
>  
>      { RUN_STATE_MAX, RUN_STATE_MAX },
>  };
> @@ -666,7 +691,9 @@ void runstate_set(RunState new_state)
>  
>  int runstate_is_running(void)
>  {
> -    return runstate_check(RUN_STATE_RUNNING);
> +    return (runstate_check(RUN_STATE_RUNNING) ||
> +            runstate_check(RUN_STATE_MIGRATE) ||
> +            runstate_check(RUN_STATE_DUMP_BITMAP));
>  }
>  
>  bool runstate_needs_reset(void)
> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK