From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38632) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLaXn-0007G3-Lw for qemu-devel@nongnu.org; Wed, 11 Feb 2015 11:53:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLaXj-0004ql-DL for qemu-devel@nongnu.org; Wed, 11 Feb 2015 11:53:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:33012) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLaXj-0004qQ-6Q for qemu-devel@nongnu.org; Wed, 11 Feb 2015 11:53:39 -0500 Date: Wed, 11 Feb 2015 16:53:33 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20150211165332.GG2371@work-vm> References: <1423584999-13946-1-git-send-email-dgilbert@redhat.com> <1423584999-13946-3-git-send-email-dgilbert@redhat.com> <87386ditwf.fsf@neno.neno> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <87386ditwf.fsf@neno.neno> Subject: Re: [Qemu-devel] [PATCH 2/3] Add migrate -u option for -incoming pause List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: amit.shah@redhat.com, pbonzini@redhat.com, liang.z.li@intel.com, qemu-devel@nongnu.org * Juan Quintela (quintela@redhat.com) wrote: > "Dr. David Alan Gilbert (git)" wrote: > > From: "Dr. David Alan Gilbert" > > > > Once a qemu has been started with -incoming pause the > > migration can be started by issuing: > > > > migrate -u uri > > > > Signed-off-by: Dr. David Alan Gilbert > > > - "(base image shared between src and destination)", > > + "(base image shared between src and destination)" > > + "\n\t\t\t -u unpauses an incoming migration started with " > > + "-incoming pause using the given uri.", > > Spaces vs tabs. > > > + -u to unpause an incoming migration started with -incoming pause > > more spaces All this is reworked anyway since I split it out. > > - qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err); > > + qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!unpause, unpause, > > + &err); > > I don't claim to understand QMP, but this whole bussines of !!foo, foo > is getting confusing, no? Yes, and it's very very easy to screw up and get them in the wrong order. > No, this is not relaced to this patch. > > > { > > Error *local_err = NULL; > > MigrationState *s = migrate_get_current(); > > @@ -450,6 +450,25 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > > return; > > } > > > > I would preffer something like: > > if (runstate_check(RUN_STATE_INMIGRATE)) { > if (unpause) { > ... unpause code > } > } else { > error_setg(errp, "Guest is waiting for an incoming migration"); > return; > } > > if (unpause) { > error_setg(errp, "Guest is waiting for an incoming migration"); > return; > } > > if (s->state == MIG_STATE_ACTIVE || s->state == MIG_STATE_SETUP || > s->state == MIG_STATE_CANCELLING) { > error_set(errp, QERR_MIGRATION_ACTIVE); > return; > } > > if (qemu_savevm_state_blocked(errp)) { > return; > } > > .... and now continue with the rest ... Again, all gone in the new version. > Thinking more about this problem, I am not sure this is the "cleanest > approach". What do you think of: > > - create RUN_STATE_INMIGRATE_PAUSED > bonus: no need of paused_incoming variable That we could do separately; doing that means carefully looking at the existing users of RUN_STATE_INMIGRATE, and since it's a visible state that includes anything that uses the interface. > - create a new migrate_incoming command > > And then we have cleaner separation of what we are doing? Done. Dave > > Later, Juan. > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK