[libvirt] [PATCH v3 1/3] Add virDomainMigrateGetMaxDowntime public API

[John Ferlan]

On 07/28/2017 10:57 AM, Scott Garfinkle wrote:
> From: Scott Garfinkle <seg at us.ibm.com>
> 

Since you'll be updating anyway from your regular email (e.g. non lotus
notes account)... Add a brief commit message here. It can be as simple
as introduce the xxx API in order to allow fetching the current downtime
value.

> ---
>  include/libvirt/libvirt-domain.h |  4 ++++
>  src/driver-hypervisor.h          |  6 ++++++
>  src/libvirt-domain.c             | 45 ++++++++++++++++++++++++++++++++++++++++
>  src/libvirt_public.syms          |  4 ++++
>  4 files changed, 59 insertions(+)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index a3bb9cb..22618b0 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -1043,6 +1043,10 @@ int virDomainMigrateSetMaxDowntime (virDomainPtr domain,
>                                      unsigned long long downtime,
>                                      unsigned int flags);
>  
> +int virDomainMigrateGetMaxDowntime (virDomainPtr domain,
                                     ^
> +                                    unsigned long long *downtime,
> +                                    unsigned int flags);
> +

I know you're just copying the Set function, but no space between e and
( - will affect the subsequent 3 lines as well.

May as well get the "new" ones right. If someone (or you) wants to fix
the existing ones in (a) separate patch(es), then more have at it!

>  int virDomainMigrateGetCompressionCache(virDomainPtr domain,
>                                          unsigned long long *cacheSize,
>                                          unsigned int flags);
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index 3053d7a..7d32cad 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -702,6 +702,11 @@ typedef int
>                                       unsigned int flags);
>  
>  typedef int
> +(*virDrvDomainMigrateGetMaxDowntime)(virDomainPtr domain,
> +                                     unsigned long long *downtime,
> +                                     unsigned int flags);
> +
> +typedef int
>  (*virDrvDomainMigrateGetCompressionCache)(virDomainPtr domain,
>                                            unsigned long long *cacheSize,
>                                            unsigned int flags);
> @@ -1412,6 +1417,7 @@ struct _virHypervisorDriver {
>      virDrvDomainGetJobInfo domainGetJobInfo;
>      virDrvDomainGetJobStats domainGetJobStats;
>      virDrvDomainAbortJob domainAbortJob;
> +    virDrvDomainMigrateGetMaxDowntime domainMigrateGetMaxDowntime;
>      virDrvDomainMigrateSetMaxDowntime domainMigrateSetMaxDowntime;
>      virDrvDomainMigrateGetCompressionCache domainMigrateGetCompressionCache;
>      virDrvDomainMigrateSetCompressionCache domainMigrateSetCompressionCache;
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 4033ae8..b78ea1c 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -8781,6 +8781,51 @@ virDomainMigrateSetMaxDowntime(virDomainPtr domain,
>  
>  
>  /**
> + * virDomainMigrateGetMaxDowntime:
> + * @domain: a domain object
> + * @downtime: return value of the maximum tolerable downtime for live
> + *            migration, in milliseconds
> + * @flags: extra flags; not used yet, so callers should always pass 0
> + *
> + * Gets current maximum tolerable time for which the domain may be paused
> + * at the end of live migration. It's supposed to be called while the domain is
> + * being live-migrated as a reaction to migration progress.

s/It's supposed ... migration progress.//

IOW: No need to cut-n-paste the 'Set' text regarding needing to be done
during the migration...  You could add something else, but I don't think
it's necessary.

> + *
> + * Returns 0 in case of success, -1 otherwise.
> + */
> +int
> +virDomainMigrateGetMaxDowntime(virDomainPtr domain,
> +                               unsigned long long *downtime,
> +                               unsigned int flags)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DOMAIN_DEBUG(domain, "downtime = %p, flags=%x", downtime, flags);
> +
> +    virResetLastError();
> +
> +    virCheckDomainReturn(domain, -1);
> +    conn = domain->conn;
> +
> +    virCheckNonNullArgGoto(downtime, error);
> +
> +    /* unlike SetMaxDowntime, it's okay for the connection to be readonly */

No need for the comment...

> +
> +    if (conn->driver->domainMigrateGetMaxDowntime) {
> +        if (conn->driver->domainMigrateGetMaxDowntime(domain,
> +                                                      downtime, flags) < 0)

Personally I would have put downtime on the previous line...

> +            goto error;
> +        return 0;
> +    }
> +
> +    virReportUnsupportedError();
> + error:
> +    virDispatchError(conn);
> +    return -1;
> +}
> +
> +
> +/**
>   * virDomainMigrateGetCompressionCache:
>   * @domain: a domain object
>   * @cacheSize: return value of current size of the cache (in bytes)
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index fac77fb..da5692a 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -768,4 +768,8 @@ LIBVIRT_3.4.0 {
>          virStreamSparseSendAll;
>  } LIBVIRT_3.1.0;
>  
> +LIBVIRT_3.6.0 {

This will be 3.7.0 at the earliest now.

John


> +    global:
> +        virDomainMigrateGetMaxDowntime;
> +} LIBVIRT_3.4.0;
>  # .... define new API here using predicted next version number ....
>