[libvirt] [PATCH v2 8/8] virsh: add postcopy-after option to migrate command

[Jiri Denemark]

On Tue, Sep 30, 2014 at 16:39:29 +0200, Cristian Klein wrote:
> Signed-off-by: Cristian Klein <cristian.klein at cs.umu.se>
> ---
>  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 36a6d52..395c73c 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -9251,6 +9251,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)")
> +    },
>      {.name = "xml",
>       .type = VSH_OT_STRING,
>       .help = N_("filename containing updated XML for the target")

We also want to add --postcopy option to enable just
VIR_MIGRATE_POSTCOPY without an automatic switch to post-copy after a
timeout. The --postcopy-after option may turn it on automatically,
though, so that users don't have to use --postcopy --postcopy-after. And
we also want to add a new migrate-startpostcopy command as a direct
wrapper to virDomainMigrateStartPostCopy.

> @@ -9332,6 +9336,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"))
> @@ -9423,6 +9429,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, 0);
> +    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)
>  {
> @@ -9432,6 +9482,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 };
>  
> @@ -9451,6 +9502,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"));

Offline migration is a migration of a domain which is not running at
all. We don't have a good name for migration of a paused domain, some
refer to it as coma migration. Anyway, this is checked for in the daemon
so checking here is redundant. Especially when this check seems to be
somewhat artificial. Sure, it doesn't make a lot of sense but post-copy
migration of a paused domain should be doable. Moreover, I believe you
can call virDomainSuspend during a post-copy migration, which is very
close to running migration without --live.

> +        goto cleanup;
> +    } else if (postCopyAfter > 0 && timeout > 0) {
> +        vshError(ctl, "%s",
> +                 _("migrate: --postcopy-after is incompatible with --timeout"));
> +        goto cleanup;
> +    }

Anyway, handling --postcopy-after seems quite complicated since we have
to repeat the logic around -1, 0, 1 return values in two places. Can't
we just directly fetch and process the value in cmdMigrate?

> +
>      if (pipe(p) < 0)
>          goto cleanup;
>  
...

Jirka