Re: [PATCH] failover: allow to pause the VM during the migration

[Juan Quintela <quintela@redhat.com>]

Laurent Vivier <lvivier@redhat.com> wrote:
> If we want to save a snapshot of a VM to a file, we used to follow the
> following steps:
>
> 1- stop the VM:
>    (qemu) stop
>
> 2- migrate the VM to a file:
>    (qemu) migrate "exec:cat > snapshot"
>
> 3- resume the VM:
>    (qemu) cont
>
> After that we can restore the snapshot with:
>   qemu-system-x86_64 ... -incoming "exec:cat snapshot"
>   (qemu) cont
>
> But when failover is configured, it doesn't work anymore.
>
> As the failover needs to ask the guest OS to unplug the card
> the machine cannot be paused.
>
> This patch introduces a new migration parameter, "pause-vm", that
> asks the migration to pause the VM during the migration startup
> phase after the the card is unplugged.
>
> Once the migration is done, we only need to resume the VM with
> "cont" and the card is plugged back:
>
> 1- set the parameter:
>    (qemu) migrate_set_parameter pause-vm on
>
> 2- migrate the VM to a file:
>    (qemu) migrate "exec:cat > snapshot"
>
>    The primary failover card (VFIO) is unplugged and the VM is paused.
>
> 3- resume the VM:
>    (qemu) cont
>
>    The VM restarts and the primary failover card is plugged back
>
> The VM state sent in the migration stream is "paused", it means
> when the snapshot is loaded or if the stream is sent to a destination
> QEMU, the VM needs to be resumed manually.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

I agree with Dave that you should use a capability instead of a
parameter.

Other than that, the code for the new parameter looks ok.
> @@ -3734,13 +3752,27 @@ static void qemu_savevm_wait_unplug(MigrationState *s, int old_state,
>                              "failure");
>              }
>          }
> -
>          migrate_set_state(&s->state, MIGRATION_STATUS_WAIT_UNPLUG, new_state);
>      } else {
>          migrate_set_state(&s->state, old_state, new_state);
>      }
>  }


This change is spurious.

And to make this more generic, I think you can consider changing the
name to pause_during_migration.

Because that is basically what managedsave needs (If I understood Laine
correctly).

Later, Juan.