From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50025) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sc8ks-0002YR-0C for qemu-devel@nongnu.org; Wed, 06 Jun 2012 01:26:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Sc8kq-00031U-7J for qemu-devel@nongnu.org; Wed, 06 Jun 2012 01:26:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56510) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Sc8kq-00031G-00 for qemu-devel@nongnu.org; Wed, 06 Jun 2012 01:26:00 -0400 Message-ID: <4FCEB6E3.4060007@redhat.com> Date: Wed, 06 Jun 2012 04:48:19 +0300 From: Orit Wasserman MIME-Version: 1.0 References: <1337691425-6022-1-git-send-email-owasserm@redhat.com> <1337691425-6022-3-git-send-email-owasserm@redhat.com> <871ulz9u5g.fsf@elfo.elfo> In-Reply-To: <871ulz9u5g.fsf@elfo.elfo> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v11 2/9] Add migration capabilites List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: quintela@redhat.com Cc: peter.maydell@linaro.org, aliguori@us.ibm.com, stefanha@gmail.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com, blauwirbel@gmail.com, chegu_vinod@hp.com, avi@redhat.com, pbonzini@redhat.com, eblake@redhat.com On 06/01/2012 01:57 PM, Juan Quintela wrote: > Orit Wasserman wrote: >> Add migration capabiltes that can be queried by the management. >> The managment can query the source QEMU and the destination QEMU in order to >> verify both support some migration capability (currently only XBZRLE). >> The managment can enable a capabilty for the next migration by using >> migrate_set_parameter command. >> >> Signed-off-by: Orit Wasserman >> +void qmp_migrate_set_parameter(const char *parameter, Error **errp) >> +{ >> + MigrationState *s = migrate_get_current(); >> + int i; >> + >> + if (s->state == MIG_STATE_ACTIVE) { >> + error_set(errp, QERR_MIGRATION_ACTIVE); >> + return; >> + } >> + >> + for (i = 0; i < MIGRATION_CAPABILITY_MAX; i++) { >> + if (strcmp(parameter, MigrationCapability_lookup[i]) == 0) { >> + s->enabled_capabilities[i] = true; >> + return; >> + } >> + } >> + >> + error_set(errp, QERR_INVALID_PARAMETER, parameter); >> +} > > Two things here: > - Is there a way to disable capabilities? it seems no. In this implementation we can't disable a capability , do you see a need to add it ? > - Would we want in the future capabilities that are not "bool"? Just > asking loud, I haven't thought a lot about this. Fixing it as a > paramenter, it would make trivial to fix previous comment: cap:true vs > cap:false, or whatever syntax we want. That is a good idea I will change it in next patch set. Orit > >> memset(s, 0, sizeof(*s)); >> s->bandwidth_limit = bandwidth_limit; >> s->params = *params; >> + memcpy(s->enabled_capabilities, enabled_capabilities, >> + sizeof(enabled_capabilities)); >> >> - s->bandwidth_limit = bandwidth_limit; >> s->state = MIG_STATE_SETUP; > > Nice catch/cleanup. > > >> diff --git a/savevm.c b/savevm.c >> index dd66f2c..42937a0 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -1711,7 +1711,7 @@ static int qemu_savevm_state(QEMUFile *f) >> int ret; >> MigrationParams params = { >> .blk = 0, >> - .shared = 0 >> + .shared = 0, >> }; >> >> if (qemu_savevm_state_blocked(NULL)) { > > This belongs to previous patch? > > Later, Juan.