[PATCH v3 7/7] qemu_driver: Add calc_mode for dirtyrate statistics

huangy81@chinatelecom.cn posted 7 patches 4 years ago
There is a newer version of this series
[PATCH v3 7/7] qemu_driver: Add calc_mode for dirtyrate statistics
Posted by huangy81@chinatelecom.cn 4 years ago
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>

Add calc_mode for dirtyrate statistics retured by
virsh domstats --dirtyrate api, also add vcpu dirtyrate
if dirty-ring mode was used in last measurement.

Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
---
 src/libvirt-domain.c         |  5 +++++
 src/qemu/qemu_driver.c       | 14 ++++++++++++
 src/qemu/qemu_monitor.h      | 10 +++++++++
 src/qemu/qemu_monitor_json.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 81 insertions(+)

diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 4caa740..afc0c1b 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -11941,6 +11941,11 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
  *     "dirtyrate.megabytes_per_second" - the calculated memory dirty rate in
  *                                        MiB/s as long long. It is produced
  *                                        only if the calc_status is measured.
+ *     "dirtyrate.calc_mode" - the calculation mode used last measurement as int,
+ *                             which is one of virDomainDirtyRateCalcMode enum.
+ *     "dirtyrate.vcpu.<num>.megabytes_per_second" - the calculated memory dirty
+ *                                                   rate for a virtual cpu as
+ *                                                   unsigned long long.
  *
  * Note that entire stats groups or individual stat fields may be missing from
  * the output in case they are not supported by the given hypervisor, are not
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5c2f289..3ef9569 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -18537,6 +18537,20 @@ qemuDomainGetStatsDirtyRate(virQEMUDriver *driver,
                                   "dirtyrate.megabytes_per_second") < 0)
         return -1;
 
+    if (virTypedParamListAddInt(params, info.mode,
+                                "dirtyrate.calc_mode") < 0)
+        return -1;
+
+    if (info.mode == VIR_DOMAIN_DIRTYRATE_CALC_MODE_DIRTY_RING) {
+        int i;
+        for (i = 0; i < info.nvcpus; i++) {
+            if (virTypedParamListAddULLong(params, info.rates[i].value,
+                                           "dirtyrate.vcpu.%d.megabytes_per_second",
+                                           info.rates[i].index) < 0)
+            return -1;
+        }
+    }
+
     return 0;
 }
 
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 6d43f23..1e500ce 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1556,6 +1556,12 @@ qemuMonitorStartDirtyRateCalc(qemuMonitor *mon,
                               int seconds,
                               virDomainDirtyRateCalcMode mode);
 
+typedef struct _qemuMonitorDirtyRateVcpu qemuMonitorDirtyRateVcpu;
+struct _qemuMonitorDirtyRateVcpu {
+    int index;                  /* virtual cpu index */
+    unsigned long long value;   /* virtual cpu dirty page rate in MB/s */
+};
+
 typedef struct _qemuMonitorDirtyRateInfo qemuMonitorDirtyRateInfo;
 struct _qemuMonitorDirtyRateInfo {
     int status;             /* the status of last dirtyrate calculation,
@@ -1563,6 +1569,10 @@ struct _qemuMonitorDirtyRateInfo {
     int calcTime;           /* the period of dirtyrate calculation */
     long long startTime;    /* the start time of dirtyrate calculation */
     long long dirtyRate;    /* the dirtyrate in MiB/s */
+    virDomainDirtyRateCalcMode mode;  /* calculation mode used in
+                                         last measurement */
+    size_t nvcpus;  /* number of virtual cpu */
+    qemuMonitorDirtyRateVcpu *rates; /* array of dirty page rate */
 };
 
 int
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8d62ca7..f088300 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -8738,11 +8738,45 @@ VIR_ENUM_IMPL(qemuMonitorDirtyRateStatus,
               "measured");
 
 static int
+qemuMonitorJSONExtractVcpuDirtyRate(virJSONValue *data,
+                                    qemuMonitorDirtyRateInfo *info)
+{
+    size_t nvcpus;
+    int i;
+
+    nvcpus = virJSONValueArraySize(data);
+    info->nvcpus = nvcpus;
+    info->rates = g_new0(qemuMonitorDirtyRateVcpu, nvcpus);
+
+    for (i = 0; i < nvcpus; i++) {
+        virJSONValue *entry = virJSONValueArrayGet(data, i);
+        if (virJSONValueObjectGetNumberInt(entry, "id",
+                                           &info->rates[i].index) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("query-dirty-rate reply was missing 'id' data"));
+            return -1;
+        }
+
+        if (virJSONValueObjectGetNumberUlong(entry, "dirty-rate",
+                                            &info->rates[i].value) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                           _("query-dirty-rate reply was missing 'dirty-rate' data"));
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+static int
 qemuMonitorJSONExtractDirtyRateInfo(virJSONValue *data,
                                     qemuMonitorDirtyRateInfo *info)
 {
     const char *statusstr;
+    const char *modestr;
     int status;
+    int mode;
+    virJSONValue *rates = NULL;
 
     if (!(statusstr = virJSONValueObjectGetString(data, "status"))) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
@@ -8779,6 +8813,24 @@ qemuMonitorJSONExtractDirtyRateInfo(virJSONValue *data,
         return -1;
     }
 
+    info->mode = VIR_DOMAIN_DIRTYRATE_CALC_MODE_PAGE_SAMPLING;
+    if ((modestr = virJSONValueObjectGetString(data, "mode"))) {
+        if ((mode = virDomainDirtyRateCalcModeTypeFromString(modestr)) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unknown dirty page rate calculation mode: %s"), modestr);
+            return -1;
+        }
+        info->mode = mode;
+    }
+
+    if ((rates = virJSONValueObjectGetArray(data, "vcpu-dirty-rate"))) {
+        if (qemuMonitorJSONExtractVcpuDirtyRate(rates, info) < 0) {
+            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+                       _("query-dirty-rate parsing 'vcpu-dirty-rate' in failure"));
+            return -1;
+        }
+    }
+
     return 0;
 }
 
-- 
1.8.3.1


Re: [PATCH v3 7/7] qemu_driver: Add calc_mode for dirtyrate statistics
Posted by Michal Prívozník 3 years, 12 months ago
On 1/28/22 08:35, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> Add calc_mode for dirtyrate statistics retured by
> virsh domstats --dirtyrate api, also add vcpu dirtyrate
> if dirty-ring mode was used in last measurement.
> 
> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> ---
>  src/libvirt-domain.c         |  5 +++++
>  src/qemu/qemu_driver.c       | 14 ++++++++++++
>  src/qemu/qemu_monitor.h      | 10 +++++++++
>  src/qemu/qemu_monitor_json.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 81 insertions(+)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 4caa740..afc0c1b 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11941,6 +11941,11 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
>   *     "dirtyrate.megabytes_per_second" - the calculated memory dirty rate in
>   *                                        MiB/s as long long. It is produced
>   *                                        only if the calc_status is measured.
> + *     "dirtyrate.calc_mode" - the calculation mode used last measurement as int,
> + *                             which is one of virDomainDirtyRateCalcMode enum.
> + *     "dirtyrate.vcpu.<num>.megabytes_per_second" - the calculated memory dirty
> + *                                                   rate for a virtual cpu as
> + *                                                   unsigned long long.
>   *
>   * Note that entire stats groups or individual stat fields may be missing from
>   * the output in case they are not supported by the given hypervisor, are not
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 5c2f289..3ef9569 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18537,6 +18537,20 @@ qemuDomainGetStatsDirtyRate(virQEMUDriver *driver,
>                                    "dirtyrate.megabytes_per_second") < 0)
>          return -1;
>  
> +    if (virTypedParamListAddInt(params, info.mode,
> +                                "dirtyrate.calc_mode") < 0)
> +        return -1;
> +
> +    if (info.mode == VIR_DOMAIN_DIRTYRATE_CALC_MODE_DIRTY_RING) {
> +        int i;
> +        for (i = 0; i < info.nvcpus; i++) {
> +            if (virTypedParamListAddULLong(params, info.rates[i].value,
> +                                           "dirtyrate.vcpu.%d.megabytes_per_second",
> +                                           info.rates[i].index) < 0)
> +            return -1;
> +        }
> +    }
> +
>      return 0;
>  }
>  
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index 6d43f23..1e500ce 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1556,6 +1556,12 @@ qemuMonitorStartDirtyRateCalc(qemuMonitor *mon,
>                                int seconds,
>                                virDomainDirtyRateCalcMode mode);
>  
> +typedef struct _qemuMonitorDirtyRateVcpu qemuMonitorDirtyRateVcpu;
> +struct _qemuMonitorDirtyRateVcpu {
> +    int index;                  /* virtual cpu index */
> +    unsigned long long value;   /* virtual cpu dirty page rate in MB/s */
> +};
> +
>  typedef struct _qemuMonitorDirtyRateInfo qemuMonitorDirtyRateInfo;
>  struct _qemuMonitorDirtyRateInfo {
>      int status;             /* the status of last dirtyrate calculation,
> @@ -1563,6 +1569,10 @@ struct _qemuMonitorDirtyRateInfo {
>      int calcTime;           /* the period of dirtyrate calculation */
>      long long startTime;    /* the start time of dirtyrate calculation */
>      long long dirtyRate;    /* the dirtyrate in MiB/s */
> +    virDomainDirtyRateCalcMode mode;  /* calculation mode used in
> +                                         last measurement */
> +    size_t nvcpus;  /* number of virtual cpu */
> +    qemuMonitorDirtyRateVcpu *rates; /* array of dirty page rate */
>  };
>  
>  int
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 8d62ca7..f088300 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -8738,11 +8738,45 @@ VIR_ENUM_IMPL(qemuMonitorDirtyRateStatus,
>                "measured");
>  
>  static int
> +qemuMonitorJSONExtractVcpuDirtyRate(virJSONValue *data,
> +                                    qemuMonitorDirtyRateInfo *info)
> +{
> +    size_t nvcpus;
> +    int i;
> +
> +    nvcpus = virJSONValueArraySize(data);
> +    info->nvcpus = nvcpus;
> +    info->rates = g_new0(qemuMonitorDirtyRateVcpu, nvcpus);
> +
> +    for (i = 0; i < nvcpus; i++) {
> +        virJSONValue *entry = virJSONValueArrayGet(data, i);
> +        if (virJSONValueObjectGetNumberInt(entry, "id",
> +                                           &info->rates[i].index) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("query-dirty-rate reply was missing 'id' data"));
> +            return -1;
> +        }
> +
> +        if (virJSONValueObjectGetNumberUlong(entry, "dirty-rate",
> +                                            &info->rates[i].value) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("query-dirty-rate reply was missing 'dirty-rate' data"));
> +            return -1;
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static int
>  qemuMonitorJSONExtractDirtyRateInfo(virJSONValue *data,
>                                      qemuMonitorDirtyRateInfo *info)
>  {
>      const char *statusstr;
> +    const char *modestr;
>      int status;
> +    int mode;
> +    virJSONValue *rates = NULL;
>  
>      if (!(statusstr = virJSONValueObjectGetString(data, "status"))) {
>          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> @@ -8779,6 +8813,24 @@ qemuMonitorJSONExtractDirtyRateInfo(virJSONValue *data,
>          return -1;
>      }
>  
> +    info->mode = VIR_DOMAIN_DIRTYRATE_CALC_MODE_PAGE_SAMPLING;

1: ^^^

> +    if ((modestr = virJSONValueObjectGetString(data, "mode"))) {
> +        if ((mode = virDomainDirtyRateCalcModeTypeFromString(modestr)) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("Unknown dirty page rate calculation mode: %s"), modestr);
> +            return -1;
> +        }
> +        info->mode = mode;
> +    }

maybe move [1] into an else branch here? Otherwise looking good.

> +
> +    if ((rates = virJSONValueObjectGetArray(data, "vcpu-dirty-rate"))) {
> +        if (qemuMonitorJSONExtractVcpuDirtyRate(rates, info) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-dirty-rate parsing 'vcpu-dirty-rate' in failure"));
> +            return -1;
> +        }
> +    }
> +
>      return 0;
>  }
>  

Michal

Re: [PATCH v3 7/7] qemu_driver: Add calc_mode for dirtyrate statistics
Posted by Daniel P. Berrangé 3 years, 12 months ago
On Fri, Jan 28, 2022 at 03:35:54PM +0800, huangy81@chinatelecom.cn wrote:
> From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> 
> Add calc_mode for dirtyrate statistics retured by
> virsh domstats --dirtyrate api, also add vcpu dirtyrate
> if dirty-ring mode was used in last measurement.
> 
> Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
> ---
>  src/libvirt-domain.c         |  5 +++++
>  src/qemu/qemu_driver.c       | 14 ++++++++++++
>  src/qemu/qemu_monitor.h      | 10 +++++++++
>  src/qemu/qemu_monitor_json.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 81 insertions(+)
> 
> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
> index 4caa740..afc0c1b 100644
> --- a/src/libvirt-domain.c
> +++ b/src/libvirt-domain.c
> @@ -11941,6 +11941,11 @@ virConnectGetDomainCapabilities(virConnectPtr conn,
>   *     "dirtyrate.megabytes_per_second" - the calculated memory dirty rate in
>   *                                        MiB/s as long long. It is produced
>   *                                        only if the calc_status is measured.
> + *     "dirtyrate.calc_mode" - the calculation mode used last measurement as int,
> + *                             which is one of virDomainDirtyRateCalcMode enum.

Definitely don't do this. For any virTypedParameter API normal practice is
to use a string to expose the data, not the rather enum integer value.

This unusual approach is why you needed to add the internal enum def
to the public header file earlier.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|