Re: [libvirt] [PATCH 6/6] virsh: added migrate --postcopy-after

On Tue, Sep 23, 2014 at 16:10:01 +0200, Cristian Klein wrote: > Added a new parameter to the `virsh migrate` command to switch to > post-copy migration after a given timeout. > > Signed-off-by: Cristian Klein <cristian.klein(a)cs.umu.se&gt; > --- > tools/virsh-domain.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > tools/virsh.pod | 5 ++++ > 2 files changed, 75 insertions(+), 2 deletions(-) > > diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c > index a6ced5f..1bba710 100644 > --- a/tools/virsh-domain.c > +++ b/tools/virsh-domain.c > @@ -9250,6 +9250,10 @@ static const vshCmdOptDef opts_migrate[] = { > .type = VSH_OT_INT, > .help = N_("force guest to suspend if live migration exceeds timeout (in seconds)") > }, > + {.name = "postcopy-after", > + .type = VSH_OT_INT, > + .help = N_("switch to post-copy migration if live migration exceeds timeout (in seconds)") > + }, I think we want both postcopy and postcopy-after options. And we should also add a dedicated migrate-startpostcopy command.

> {.name = "xml", > .type = VSH_OT_STRING, > .help = N_("filename containing updated XML for the target") > @@ -9331,6 +9335,8 @@ doMigrate(void *opaque) > VIR_FREE(xml); > } > > + if (vshCommandOptBool(cmd, "postcopy-after")) /* actually an int */ > + flags |= VIR_MIGRATE_POSTCOPY; > if (vshCommandOptBool(cmd, "live")) > flags |= VIR_MIGRATE_LIVE; > if (vshCommandOptBool(cmd, "p2p")) > @@ -9422,6 +9428,50 @@ vshMigrationTimeout(vshControl *ctl, > virDomainSuspend(dom); > } > > +static void > +vshMigrationPostCopyAfter(vshControl *ctl, > + virDomainPtr dom, > + void *opaque ATTRIBUTE_UNUSED) > +{ > + vshDebug(ctl, VSH_ERR_DEBUG, "starting post-copy\n"); > + int rv = virDomainMigrateStartPostCopy(dom); > + if (rv < 0) { > + vshError(ctl, "%s", _("start post-copy command failed")); > + } else { > + vshDebug(ctl, VSH_ERR_INFO, "switched to post-copy\n"); > + } > +} > + > +/* Parse the --postcopy-after parameter in seconds, but store its > + * value in milliseconds. Return -1 on error, 0 if > + * no timeout was requested, and 1 if timeout was set. > + * Copy-paste-adapted from vshCommandOptTimeoutToMs. > + */ > +static int > +vshCommandOptPostCopyAfterToMs(vshControl *ctl, const vshCmd *cmd, int *postCopyAfter) > +{ > + int rv = vshCommandOptInt(cmd, "postcopy-after", postCopyAfter); > + > + if (rv < 0 || (rv > 0 && *postCopyAfter < 0)) { > + vshError(ctl, "%s", _("invalid postcopy-after parameter")); > + return -1; > + } > + if (rv > 0) { > + /* Ensure that we can multiply by 1000 without overflowing. */ > + if (*postCopyAfter > INT_MAX / 1000) { > + vshError(ctl, "%s", _("post-copy after parameter is too big")); > + return -1; > + } > + *postCopyAfter *= 1000; > + /* 0 is a special value inside virsh, which means no timeout, so > + * use 1ms instead for "start post-copy immediately" > + */ > + if (*postCopyAfter == 0) > + *postCopyAfter = 1; > + } > + return rv; > +} > + > static bool > cmdMigrate(vshControl *ctl, const vshCmd *cmd) > { > @@ -9431,6 +9481,7 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) > bool verbose = false; > bool functionReturn = false; > int timeout = 0; > + int postCopyAfter = 0; > bool live_flag = false; > vshCtrlData data = { .dconn = NULL }; > > @@ -9450,6 +9501,18 @@ cmdMigrate(vshControl *ctl, const vshCmd *cmd) > goto cleanup; > } > > + if (vshCommandOptPostCopyAfterToMs(ctl, cmd, &postCopyAfter) < 0) { > + goto cleanup; > + } else if (postCopyAfter > 0 && !live_flag) { > + vshError(ctl, "%s", > + _("migrate: Unexpected postcopy-after for offline migration")); > + goto cleanup; > + } else if (postCopyAfter > 0 && timeout > 0) { > + vshError(ctl, "%s", > + _("migrate: --postcopy-after is incompatible with --timeout")); > + goto cleanup; > + } > + Is post-copy only allowed for live migrations? I know it doesn't make much sense otherwise but I'm just checking... Anyway, this check belongs to qemu driver rather than virsh so that any API user gets the error message.