[PATCH v4 5/7] migration/dirtyrate: Implement qemuMonitorJSONExtractDirtyRateInfo

[Michal Privoznik]

On 11/7/20 10:41 AM, Hao Wang wrote:
> Implement qemuMonitorJSONExtractDirtyRateInfo to deal with the return from
> qmp "query-dirty-rate", and store them in virDomainDirtyRateInfo.
> 
> Signed-off-by: Hao Wang <wanghao232 at huawei.com>
> Reviewed-by: Chuan Zheng <zhengchuan at huawei.com>
> ---
>   include/libvirt/libvirt-domain.h | 17 +++++++++
>   src/qemu/qemu_monitor_json.c     | 59 ++++++++++++++++++++++++++++++--
>   2 files changed, 73 insertions(+), 3 deletions(-)
> 
> diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h
> index b950736b67..51d8685086 100644
> --- a/include/libvirt/libvirt-domain.h
> +++ b/include/libvirt/libvirt-domain.h
> @@ -5096,6 +5096,23 @@ int virDomainBackupBegin(virDomainPtr domain,
>   char *virDomainBackupGetXMLDesc(virDomainPtr domain,
>                                   unsigned int flags);
>   
> +/**
> + * virDomainDirtyRateStatus:
> + *
> + * Details on the cause of a dirtyrate 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_MEASURED  = 2, /* the dirtyrate calculation is
> +completed */
> +
> +# ifdef VIR_ENUM_SENTINELS
> +    VIR_DOMAIN_DIRTYRATE_LAST
> +# endif
> +} virDomainDirtyRateStatus;
> +

As advertised earlier, this doesn't belong into this commit really.

>   /**
>    * virDomainDirtyRateInfo:
>    *
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 1924c7229b..ca7d8d23c0 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -9659,12 +9659,61 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
>   }
>   
>   
> +VIR_ENUM_DECL(qemuDomainDirtyRateStatus);
> +VIR_ENUM_IMPL(qemuDomainDirtyRateStatus,
> +              VIR_DOMAIN_DIRTYRATE_LAST,
> +              "unstarted",
> +              "measuring",
> +              "measured");
> +
> +static int
> +qemuMonitorJSONExtractDirtyRateInfo(virJSONValuePtr data,
> +                                    virDomainDirtyRateInfoPtr info)
> +{
> +    const char *status;
> +    int statusID;
> +
> +    if (!(status = virJSONValueObjectGetString(data, "status"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-dirty-rate reply was missing 'status' data"));
> +        return -1;
> +    }
> +
> +    if ((statusID = qemuDomainDirtyRateStatusTypeFromString(status)) < 0) {
> +        return -1;

So if qemu sends us some other string, this fails silently.

> +    }
> +    info->status = statusID;
> +
> +    if ((info->status == VIR_DOMAIN_DIRTYRATE_MEASURED) &&
> +        (virJSONValueObjectGetNumberLong(data, "dirty-rate", &(info->dirtyRate)) < 0)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-dirty-rate reply was missing 'dirty-rate' data"));
> +        return -1;
> +    }
> +
> +    if (virJSONValueObjectGetNumberLong(data, "start-time", &(info->startTime)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-dirty-rate reply was missing 'start-time' data"));
> +        return -1;
> +    }
> +
> +    if (virJSONValueObjectGetNumberLong(data, "calc-time", &(info->calcTime)) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-dirty-rate reply was missing 'calc-time' data"));
> +        return -1;
> +    }
> +
> +    return 0;
> +}
> +
> +
>   int
>   qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
>                                 virDomainDirtyRateInfoPtr info)
>   {
>       g_autoptr(virJSONValue) cmd = NULL;
>       g_autoptr(virJSONValue) reply = NULL;
> +    virJSONValuePtr data = NULL;
>   
>       if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL)))
>           return -1;
> @@ -9675,7 +9724,11 @@ qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
>       if (qemuMonitorJSONCheckError(cmd, reply) < 0)
>           return -1;
>   
> -    /* TODO: extract dirtyrate data from reply and store in virDomainDirtyRateInfoPtr */
> -    info->status = 0;
> -    return 0;
> +    if (!(data = virJSONValueObjectGetObject(reply, "return"))) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("query-dirty-rate reply was missing 'return' data"));
> +        return -1;
> +    }
> +
> +    return qemuMonitorJSONExtractDirtyRateInfo(data, info);
>   }
> 

I think the following should be squashed in:

diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 78ad10bc24..4715b33254 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9680,24 +9680,26 @@ 
qemuMonitorJSONExtractDirtyRateInfo(virJSONValuePtr data,
      }

      if ((statusID = qemuDomainDirtyRateStatusTypeFromString(status)) < 
0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unknown dirty rate status: %s"), status);
          return -1;
      }
      info->status = statusID;

      if ((info->status == VIR_DOMAIN_DIRTYRATE_MEASURED) &&
-        (virJSONValueObjectGetNumberLong(data, "dirty-rate", 
&(info->dirtyRate)) < 0)) {
+        (virJSONValueObjectGetNumberLong(data, "dirty-rate", 
&info->dirtyRate) < 0)) {
          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                         _("query-dirty-rate reply was missing 
'dirty-rate' data"));
          return -1;
      }

-    if (virJSONValueObjectGetNumberLong(data, "start-time", 
&(info->startTime)) < 0) {
+    if (virJSONValueObjectGetNumberLong(data, "start-time", 
&info->startTime) < 0) {
          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                         _("query-dirty-rate reply was missing 
'start-time' data"));
          return -1;
      }

-    if (virJSONValueObjectGetNumberLong(data, "calc-time", 
&(info->calcTime)) < 0) {
+    if (virJSONValueObjectGetNumberLong(data, "calc-time", 
&info->calcTime) < 0) {
          virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                         _("query-dirty-rate reply was missing 
'calc-time' data"));
          return -1;


Michal