[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v4 3/4] virsh: Add support for virDomainMigrateGetMaxDowntime




On 08/17/2017 06:17 PM, Scott Garfinkle wrote:
> Implement a migrate-getmaxdowntime command to complement migrate-setmaxdowntime.
> ---
>  tools/virsh-domain.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  tools/virsh.pod      |  6 ++++++
>  2 files changed, 48 insertions(+)
> 

Please don't forget to post a patch to add the migrate-compcache that
you rightly removed from this particular patch since it was "missing"
from some other original patch.

> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 40f1673..012c96e 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -10719,6 +10719,42 @@ cmdMigrateSetMaxDowntime(vshControl *ctl, const vshCmd *cmd)
>      return ret;
>  }
>  
> + /*

Bad indent here.

> + * "migrate-getmaxdowntime" command
> + */
> +static const vshCmdInfo info_migrate_getmaxdowntime[] = {
> +    {.name = "help",
> +     .data = N_("get maximum tolerable downtime")
> +    },
> +    {.name = "desc",
> +     .data = N_("Get maximum tolerable downtime of a domain which is being live-migrated to another host.")
> +    },
> +    {.name = NULL}
> +};
> +
> +static const vshCmdOptDef opts_migrate_getmaxdowntime[] = {
> +    VIRSH_COMMON_OPT_DOMAIN_FULL,
> +    {.name = NULL}
> +};
> +
> +static bool
> +cmdMigrateGetMaxDowntime(vshControl *ctl, const vshCmd *cmd)
> +{
> +    virDomainPtr dom;
> +    unsigned long long downtime;
> +    bool ret = false;
> +
> +    if ((dom = virshCommandOptDomain(ctl, cmd, NULL)) != NULL) {
> +        if (virDomainMigrateGetMaxDowntime(dom, &downtime, 0) >= 0) {
> +            vshPrint(ctl, "%llu\n", downtime);
> +            ret = true;
> +        }
> +        virshDomainFree(dom);
> +    }
> +
> +    return ret;

Although technically correct, the keeping the style of function logic is
preferred, thus I'll adjust to:

    if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
        return false;

    if (virDomainMigrateGetMaxDowntime(dom, &downtime, 0) < 0)
        goto done;

    vshPrint(ctl, "%llu\n", downtime);
    ret = true;

 done:
    virshDomainFree(dom);
    return ret;

I'll adjust the code before pushing


Reviewed-by: John Ferlan <jferlan redhat com>

John

> +}
> +
>  /*
>   * "migrate-compcache" command
>   */
> @@ -13875,6 +13911,12 @@ const vshCmdDef domManagementCmds[] = {
>       .info = info_migrate_setmaxdowntime,
>       .flags = 0
>      },
> +    {.name = "migrate-getmaxdowntime",
> +     .handler = cmdMigrateGetMaxDowntime,
> +     .opts = opts_migrate_getmaxdowntime,
> +     .info = info_migrate_getmaxdowntime,
> +     .flags = 0
> +    },
>      {.name = "migrate-compcache",
>       .handler = cmdMigrateCompCache,
>       .opts = opts_migrate_compcache,
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 43d6f0c..15a4143 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -1857,6 +1857,12 @@ Set maximum tolerable downtime for a domain which is being live-migrated to
>  another host.  The I<downtime> is a number of milliseconds the guest is allowed
>  to be down at the end of live migration.
>  
> +=item B<migrate-getmaxdowntime> I<domain>
> +
> +Get the maximum tolerable downtime for a domain which is being live-migrated to
> +another host.  This is the number of milliseconds the guest is allowed
> +to be down at the end of live migration.
> +
>  =item B<migrate-compcache> I<domain> [I<--size> B<bytes>]
>  
>  Sets and/or gets size of the cache (in bytes) used for compressing repeatedly
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]