[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] qmp: handle stop/cont in INMIGRATE state
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] qmp: handle stop/cont in INMIGRATE state |
Date: |
Mon, 22 Oct 2012 13:06:37 -0200 |
On Fri, 19 Oct 2012 16:45:23 +0200
Paolo Bonzini <address@hidden> wrote:
> Right now, stop followed by an incoming migration will let the
> virtual machine start. cont before an incoming migration instead
> will fail.
>
> This is bad because the actual behavior is not predictable; it is
> racy with respect to the start of the incoming migration. That's
> because incoming migration is blocking, and thus will delay the
> processing of stop/cont until the end of the migration.
>
> In addition, there's nothing that really prevents the user from
> typing the block device's passwords before incoming migration is
> done, so returning the DeviceEncrypted error is also helpful in
> the QMP case.
>
> Both things can be fixed by just toggling the autostart variable when
> stop/cont are called in INMIGRATE state.
>
> Note that libvirt is currently working around the race by looping
> if the MigrationExpected answer is returned. After this patch, the
> command will return right away without ever raising an error.
>
> Signed-off-by: Paolo Bonzini <address@hidden>
Could you please add a note in cont's and stop's entries in qapi-schema.json
explaining their behavior on INMIGRATE?
Otherwise series looks good.
> ---
> v1->v2: kill MigrationExpected and QERR_MIGRATION_EXPECTED, yay!
>
> qapi-schema.json | 7 +------
> qerror.h | 3 ---
> qmp.c | 17 +++++++++++------
> 3 file modificati, 12 inserzioni(+), 15 rimozioni(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index c615ee2..7e102c5 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -22,15 +22,11 @@
> # @KVMMissingCap: the requested operation can't be fulfilled because a
> # required KVM capability is missing
> #
> -# @MigrationExpected: the requested operation can't be fulfilled because a
> -# migration process is expected
> -#
> # Since: 1.2
> ##
> { 'enum': 'ErrorClass',
> 'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
> - 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap',
> - 'MigrationExpected' ] }
> + 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
>
> ##
> # @add_client
> @@ -1299,7 +1295,6 @@
> # Since: 0.14.0
> #
> # Returns: If successful, nothing
> -# If the QEMU is waiting for an incoming migration,
> MigrationExpected
> # If QEMU was started with an encrypted block device and a key has
> # not yet been set, DeviceEncrypted.
> #
> diff --git a/qerror.h b/qerror.h
> index c91708c..5e98a39 100644
> --- a/qerror.h
> +++ b/qerror.h
> @@ -165,9 +165,6 @@ void assert_no_error(Error *err);
> #define QERR_MIGRATION_NOT_SUPPORTED \
> ERROR_CLASS_GENERIC_ERROR, "State blocked by non-migratable device '%s'"
>
> -#define QERR_MIGRATION_EXPECTED \
> - ERROR_CLASS_MIGRATION_EXPECTED, "An incoming migration is expected
> before this command can be executed"
> -
> #define QERR_MISSING_PARAMETER \
> ERROR_CLASS_GENERIC_ERROR, "Parameter '%s' is missing"
>
> diff --git a/qmp.c b/qmp.c
> index 36c54c5..2c8d559 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -85,7 +85,11 @@ void qmp_quit(Error **err)
>
> void qmp_stop(Error **errp)
> {
> - vm_stop(RUN_STATE_PAUSED);
> + if (runstate_check(RUN_STATE_INMIGRATE)) {
> + autostart = 0;
> + } else {
> + vm_stop(RUN_STATE_PAUSED);
> + }
> }
>
> void qmp_system_reset(Error **errp)
> @@ -144,10 +148,7 @@ void qmp_cont(Error **errp)
> {
> Error *local_err = NULL;
>
> - if (runstate_check(RUN_STATE_INMIGRATE)) {
> - error_set(errp, QERR_MIGRATION_EXPECTED);
> - return;
> - } else if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
> + if (runstate_check(RUN_STATE_INTERNAL_ERROR) ||
> runstate_check(RUN_STATE_SHUTDOWN)) {
> error_set(errp, QERR_RESET_REQUIRED);
> return;
> @@ -162,7 +163,11 @@ void qmp_cont(Error **errp)
> return;
> }
>
> - vm_start();
> + if (runstate_check(RUN_STATE_INMIGRATE)) {
> + autostart = 1;
> + } else {
> + vm_start();
> + }
> }
>
> void qmp_system_wakeup(Error **errp)