From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45319) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLE9e-0001cg-3x for qemu-devel@nongnu.org; Tue, 10 Feb 2015 11:59:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLE9Y-0007uh-K2 for qemu-devel@nongnu.org; Tue, 10 Feb 2015 11:59:18 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50959) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLE9Y-0007uC-Dj for qemu-devel@nongnu.org; Tue, 10 Feb 2015 11:59:12 -0500 Date: Tue, 10 Feb 2015 16:59:06 +0000 From: "Dr. David Alan Gilbert" Message-ID: <20150210165905.GE2376@work-vm> References: <1423584999-13946-1-git-send-email-dgilbert@redhat.com> <1423584999-13946-3-git-send-email-dgilbert@redhat.com> <20150210164720.GK400@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150210164720.GK400@redhat.com> 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: "Daniel P. Berrange" Cc: quintela@redhat.com, liang.z.li@intel.com, qemu-devel@nongnu.org, amit.shah@redhat.com, pbonzini@redhat.com * Daniel P. Berrange (berrange@redhat.com) wrote: > On Tue, Feb 10, 2015 at 04:16:38PM +0000, 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 > > --- > > hmp-commands.hx | 11 +++++++---- > > hmp.c | 6 ++++-- > > migration/migration.c | 21 ++++++++++++++++++++- > > qapi-schema.json | 5 ++++- > > qmp-commands.hx | 3 ++- > > 5 files changed, 37 insertions(+), 9 deletions(-) > > > > diff --git a/hmp-commands.hx b/hmp-commands.hx > > index e37bc8b..45f293a 100644 > > --- a/hmp-commands.hx > > +++ b/hmp-commands.hx > > @@ -887,23 +887,26 @@ ETEXI > > > > { > > .name = "migrate", > > - .args_type = "detach:-d,blk:-b,inc:-i,uri:s", > > - .params = "[-d] [-b] [-i] uri", > > + .args_type = "detach:-d,blk:-b,inc:-i,unpause:-u,uri:s", > > + .params = "[-d] [-b] [-i] [-u] uri", > > .help = "migrate to URI (using -d to not wait for completion)" > > "\n\t\t\t -b for migration without shared storage with" > > " full copy of disk\n\t\t\t -i for migration without " > > "shared storage with incremental copy of disk " > > - "(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.", > > .mhandler.cmd = hmp_migrate, > > }, > > > > > > STEXI > > -@item migrate [-d] [-b] [-i] @var{uri} > > +@item migrate [-d] [-b] [-i] [-u] @var{uri} > > @findex migrate > > Migrate to @var{uri} (using -d to not wait for completion). > > -b for migration with full copy of disk > > -i for migration with incremental copy of disk (base image is shared) > > + -u to unpause an incoming migration started with -incoming pause > > ETEXI > > > > { > > diff --git a/hmp.c b/hmp.c > > index b47f331..fb0cde1 100644 > > --- a/hmp.c > > +++ b/hmp.c > > @@ -1344,17 +1344,19 @@ void hmp_migrate(Monitor *mon, const QDict *qdict) > > int detach = qdict_get_try_bool(qdict, "detach", 0); > > int blk = qdict_get_try_bool(qdict, "blk", 0); > > int inc = qdict_get_try_bool(qdict, "inc", 0); > > + int unpause = qdict_get_try_bool(qdict, "unpause", 0); > > const char *uri = qdict_get_str(qdict, "uri"); > > Error *err = NULL; > > > > - qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err); > > + qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, !!unpause, unpause, > > + &err); > > if (err) { > > monitor_printf(mon, "migrate: %s\n", error_get_pretty(err)); > > error_free(err); > > return; > > } > > > > - if (!detach) { > > + if (!detach && !unpause) { > > MigrationStatus *status; > > > > if (monitor_suspend(mon) < 0) { > > diff --git a/migration/migration.c b/migration/migration.c > > index 77263a5..2308067 100644 > > --- a/migration/migration.c > > +++ b/migration/migration.c > > @@ -434,7 +434,7 @@ void migrate_del_blocker(Error *reason) > > > > void qmp_migrate(const char *uri, bool has_blk, bool blk, > > bool has_inc, bool inc, bool has_detach, bool detach, > > - Error **errp) > > + bool has_unpause, bool unpause, Error **errp) > > { > > 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; > > } > > > > + if (unpause) { > > + /* Unpause is starting up an *incoming* migration */ > > + if (detach || inc || blk) { > > + error_setg(errp, "Other flags are not meaningful for unpause"); > > + return; > > + } > > + > > + if (!paused_incoming) { > > + error_setg(errp, "Unpause can only be used with -incoming pause"); > > + return; > > + } > > + > > + qemu_start_incoming_migration(uri, errp); > > + if (!errp || !*errp) { > > + paused_incoming = false; > > + } > > + return; > > + } > > + > > if (runstate_check(RUN_STATE_INMIGRATE)) { > > error_setg(errp, "Guest is waiting for an incoming migration"); > > return; > > Hmm, the 'unpause' codepath doesn't really share anything with the existing > codepath. Also the URIs for the existing migrate command are not quite the > same as the URIs for the incoming migrate side. This would suggest to me > that it might be better to have a separate 'migrate-incoming' command in > the monitor rather than overload the existing 'migrate' command. > > Also having a separate command will make it possible to detect that this > feature is supported from libvirt, since I don't think QMP introspection > provides enough info to detect it based on the new arg to existing > commands. OK, that would be easy enough for me to split out. I'm not completely convinced that the URI parsing is currently different between incoming and migrate; for example the 'migrate' command seems to blindly take ,to=4445 which I think it then ignores. But that's more of a bug. Dave > > > Regards, > Daniel > -- > |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK