From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:60221) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQJZf-0004NX-6G for qemu-devel@nongnu.org; Mon, 22 Oct 2012 11:05:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQJZW-0001Ta-B2 for qemu-devel@nongnu.org; Mon, 22 Oct 2012 11:05:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:22382) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQJZV-0001TA-Rs for qemu-devel@nongnu.org; Mon, 22 Oct 2012 11:05:42 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q9MF5f1Z020166 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 22 Oct 2012 11:05:41 -0400 Date: Mon, 22 Oct 2012 13:06:37 -0200 From: Luiz Capitulino Message-ID: <20121022130637.723ccc6a@doriath.home> In-Reply-To: <1350657924-10624-2-git-send-email-pbonzini@redhat.com> References: <1350657924-10624-1-git-send-email-pbonzini@redhat.com> <1350657924-10624-2-git-send-email-pbonzini@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/2] qmp: handle stop/cont in INMIGRATE state List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: qemu-devel@nongnu.org On Fri, 19 Oct 2012 16:45:23 +0200 Paolo Bonzini 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 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)