[PATCH v4 2/7] migration/dirtyrate: set up framwork of domainGetDirtyRateInfo API

Michal Privoznik mprivozn at redhat.com
Tue Nov 10 20:11:42 UTC 2020


On 11/7/20 10:41 AM, Hao Wang wrote:
> Introduce DomainGetDirtyRateInfo API for domain's memory dirty rate
> calculation and query.
> 
> Signed-off-by: Hao Wang <wanghao232 at huawei.com>
> Signed-off-by: Zhou Yimin <zhouyimin at huawei.com>
> Reviewed-by: Chuan Zheng <zhengchuan at huawei.com>
> ---
>   include/libvirt/libvirt-domain.h |  5 ++++
>   src/driver-hypervisor.h          |  7 +++++
>   src/libvirt-domain.c             | 46 ++++++++++++++++++++++++++++++++
>   src/libvirt_public.syms          |  5 ++++
>   src/remote/remote_driver.c       |  1 +
>   src/remote/remote_protocol.x     | 21 ++++++++++++++-
>   6 files changed, 84 insertions(+), 1 deletion(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index 145f517068..b950736b67 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -5120,4 +5120,9 @@ struct _virDomainDirtyRateInfo {
>   
>   typedef virDomainDirtyRateInfo *virDomainDirtyRateInfoPtr;
>   
> +int virDomainGetDirtyRateInfo(virDomainPtr domain,
> +                              virDomainDirtyRateInfoPtr info,
> +                              long long sec,
> +                              unsigned int flags);
> +
>   #endif /* LIBVIRT_DOMAIN_H */
> diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h
> index bce023017d..a77c29de54 100644
> --- a/src/driver-hypervisor.h
> +++ b/src/driver-hypervisor.h
> @@ -1387,6 +1387,12 @@ typedef char *
>   (*virDrvDomainBackupGetXMLDesc)(virDomainPtr domain,
>                                   unsigned int flags);
>   
> +typedef int
> +(*virDrvDomainGetDirtyRateInfo)(virDomainPtr domain,
> +                                virDomainDirtyRateInfoPtr info,
> +                                long long sec,
> +                                unsigned int flags);
> +
>   typedef struct _virHypervisorDriver virHypervisorDriver;
>   typedef virHypervisorDriver *virHypervisorDriverPtr;
>   
> @@ -1650,4 +1656,5 @@ struct _virHypervisorDriver {
>       virDrvDomainAgentSetResponseTimeout domainAgentSetResponseTimeout;
>       virDrvDomainBackupBegin domainBackupBegin;
>       virDrvDomainBackupGetXMLDesc domainBackupGetXMLDesc;
> +    virDrvDomainGetDirtyRateInfo domainGetDirtyRateInfo;
>   };
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 3c5f55176a..ce3c40edf8 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -12758,3 +12758,49 @@ virDomainBackupGetXMLDesc(virDomainPtr domain,
>       virDispatchError(conn);
>       return NULL;
>   }
> +
> +
> +/**
> + * virDomainGetDirtyRateInfo:
> + * @domain: a domain object.
> + * @info: return value of current domain's memory dirty rate info.
> + * @sec: show dirty rate within specified seconds.
> + * @flags: the flags of getdirtyrate action -- calculate and/or query.

What are the flags? Which enum should I look at?

> + *
> + * Get the current domain's memory dirty rate (in MB/s).

Can you expand on this a bit? Look at description to other APIs for 
inspiration.

> + *
> + * Returns 0 in case of success, -1 otherwise.
> + */
> +int
> +virDomainGetDirtyRateInfo(virDomainPtr domain,
> +                          virDomainDirtyRateInfoPtr info,
> +                          long long sec,

Do we really need long long? That's more than 2.9*10^11 years. I don't 
think any virtual machine will ever run for so long (considering that 
the age of the universe is ~1.38*10^10 years)

> +                          unsigned int flags)
> +{
> +    virConnectPtr conn;
> +
> +    VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);

@flags should also be logged.

> +
> +    virResetLastError();
> +
> +    virCheckDomainReturn(domain, -1);
> +    conn = domain->conn;
> +
> +    virCheckNonNullArgGoto(info, error);
> +    virCheckReadOnlyGoto(conn->flags, error);
> +
> +    if (info)
> +        memset(info, 0, sizeof(*info));

@info was checked for NULL just two lines above.

> +
> +    if (conn->driver->domainGetDirtyRateInfo) {
> +        if (conn->driver->domainGetDirtyRateInfo(domain, info, sec, flags) < 0)
> +            goto error;
> +        VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);

This is not very useful debug log.

> +        return 0;
> +    }
> +
> +    virReportUnsupportedError();
> + error:
> +    virDispatchError(conn);
> +    return -1;
> +}
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 539d2e3943..11864f48b1 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -873,4 +873,9 @@ LIBVIRT_6.0.0 {
>           virDomainBackupGetXMLDesc;
>   } LIBVIRT_5.10.0;
>   
> +LIBVIRT_6.9.0 {
> +    global:
> +        virDomainGetDirtyRateInfo;
> +} LIBVIRT_6.0.0;
> +

Unfortunately, this missed 6.9.0 so this must be 6.10.0 instead.

>   # .... define new API here using predicted next version number ....
> diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
> index 9cd2fd36ae..325968a22b 100644
> --- a/src/remote/remote_driver.c
> +++ b/src/remote/remote_driver.c
> @@ -8458,6 +8458,7 @@ static virHypervisorDriver hypervisor_driver = {
>       .domainAgentSetResponseTimeout = remoteDomainAgentSetResponseTimeout, /* 5.10.0 */
>       .domainBackupBegin = remoteDomainBackupBegin, /* 6.0.0 */
>       .domainBackupGetXMLDesc = remoteDomainBackupGetXMLDesc, /* 6.0.0 */
> +    .domainGetDirtyRateInfo = remoteDomainGetDirtyRateInfo, /* 6.9.0 */

Same here.

>   };
>   
>   static virNetworkDriver network_driver = {


I think the following should be squashed in:

diff --git a/include/libvirt/libvirt-domain.h 
b/include/libvirt/libvirt-domain.h
index e8bd890b29..594d7a2790 100644
--- a/include/libvirt/libvirt-domain.h
+++ b/include/libvirt/libvirt-domain.h
@@ -5096,28 +5096,18 @@ int virDomainBackupBegin(virDomainPtr domain,
  char *virDomainBackupGetXMLDesc(virDomainPtr domain,
                                  unsigned int flags);

-/**
- * virDomainDirtyRateFlags:
- *
- * Details on the flags used by getdirtyrate api.
- */
-
-typedef enum {
-    VIR_DOMAIN_DIRTYRATE_CALC = 1 << 0,  /* calculate domain's dirtyrate */
-    VIR_DOMAIN_DIRTYRATE_QUERY = 1 << 1, /* query domain's dirtyrate */
-} virDomainDirtyRateFlags;
-
  /**
   * virDomainDirtyRateStatus:
   *
- * Details on the cause of a dirtyrate calculation status.
+ * Details on the cause of a dirty rate calculation status.
   */
-
  typedef enum {
-    VIR_DOMAIN_DIRTYRATE_UNSTARTED = 0, /* the dirtyrate calculation 
has not been started */
-    VIR_DOMAIN_DIRTYRATE_MEASURING = 1, /* the dirtyrate calculation is 
measuring */
+    VIR_DOMAIN_DIRTYRATE_UNSTARTED = 0, /* the dirtyrate calculation 
has not
+                                           been started */
+    VIR_DOMAIN_DIRTYRATE_MEASURING = 1, /* the dirtyrate calculation is
+                                           measuring */
      VIR_DOMAIN_DIRTYRATE_MEASURED  = 2, /* the dirtyrate calculation is
-completed */
+                                           completed */

  # ifdef VIR_ENUM_SENTINELS
      VIR_DOMAIN_DIRTYRATE_LAST
@@ -5133,12 +5123,23 @@ completed */
  typedef struct _virDomainDirtyRateInfo virDomainDirtyRateInfo;
  typedef virDomainDirtyRateInfo *virDomainDirtyRateInfoPtr;
  struct _virDomainDirtyRateInfo {
-    int status;             /* the status of dirty rate calculation */
+    int status;             /* the status of dirty rate calculation, one of
+                               virDomainDirtyRateStatus */
      long long dirtyRate;    /* the dirty rate in MB/s */
      long long startTime;    /* the start time of dirty rate calculation */
      long long calcTime;     /* the period of dirty rate calculation */
  };

+/**
+ * virDomainDirtyRateFlags:
+ *
+ * Details on the flags used by getdirtyrate api.
+ */
+typedef enum {
+    VIR_DOMAIN_DIRTYRATE_CALC = 1 << 0,  /* calculate domain's dirty 
rate */
+    VIR_DOMAIN_DIRTYRATE_QUERY = 1 << 1, /* query domain's dirty rate */
+} virDomainGetDirtyRateInfoFlags;
+
  int virDomainGetDirtyRateInfo(virDomainPtr domain,
                                virDomainDirtyRateInfoPtr info,
                                long long sec,
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index ce3c40edf8..75029b0a4b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -12762,10 +12762,10 @@ virDomainBackupGetXMLDesc(virDomainPtr domain,

  /**
   * virDomainGetDirtyRateInfo:
- * @domain: a domain object.
- * @info: return value of current domain's memory dirty rate info.
- * @sec: show dirty rate within specified seconds.
- * @flags: the flags of getdirtyrate action -- calculate and/or query.
+ * @domain: a domain object
+ * @info: pointer to current domain's memory dirty rate info
+ * @sec: show dirty rate within specified seconds
+ * @flags: extra flags; binary-OR of virDomainGetDirtyRateInfoFlags
   *
   * Get the current domain's memory dirty rate (in MB/s).
   *
@@ -12779,7 +12779,8 @@ virDomainGetDirtyRateInfo(virDomainPtr domain,
  {
      virConnectPtr conn;

-    VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);
+    VIR_DOMAIN_DEBUG(domain, "info=%p, sec=%lld, flags=0x%x",
+                     info, sec, flags);

      virResetLastError();

@@ -12789,14 +12790,14 @@ virDomainGetDirtyRateInfo(virDomainPtr domain,
      virCheckNonNullArgGoto(info, error);
      virCheckReadOnlyGoto(conn->flags, error);

-    if (info)
-        memset(info, 0, sizeof(*info));
+    memset(info, 0, sizeof(*info));

      if (conn->driver->domainGetDirtyRateInfo) {
-        if (conn->driver->domainGetDirtyRateInfo(domain, info, sec, 
flags) < 0)
+        int ret;
+        ret = conn->driver->domainGetDirtyRateInfo(domain, info, sec, 
flags);
+        if (ret < 0)
              goto error;
-        VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);
-        return 0;
+        return ret;
      }

      virReportUnsupportedError();

Michal




More information about the libvir-list mailing list