[PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate

[Michal Privoznik]

On 11/7/20 10:41 AM, Hao Wang wrote:
> Implement qemuDomainQueryDirtyRate which query domain's memory
> dirty rate calling qmp "query-dirty-rate".
> 
> 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>
> ---
>   src/qemu/qemu_migration.c    | 31 +++++++++++++++++++++++++++++++
>   src/qemu/qemu_migration.h    |  5 +++++
>   src/qemu/qemu_monitor.c      | 12 ++++++++++++
>   src/qemu/qemu_monitor.h      |  4 ++++
>   src/qemu/qemu_monitor_json.c | 22 ++++++++++++++++++++++
>   src/qemu/qemu_monitor_json.h |  4 ++++
>   6 files changed, 78 insertions(+)
> 
> diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
> index 8029e24415..3d07ba3ac4 100644
> --- a/src/qemu/qemu_migration.c
> +++ b/src/qemu/qemu_migration.c
> @@ -5864,3 +5864,34 @@ qemuDomainCalculateDirtyRate(virDomainPtr dom,
>   
>       return ret;
>   }
> +
> +
> +int
> +qemuDomainQueryDirtyRate(virDomainPtr dom,
> +                         virDomainObjPtr vm,
> +                         virDomainDirtyRateInfoPtr info)
> +{
> +    virQEMUDriverPtr driver = dom->conn->privateData;

Again, driver is accessible from the caller, just pass it directly.

> +    qemuDomainObjPrivatePtr priv;
> +    int ret = -1;
> +
> +    if (!virDomainObjIsActive(vm)) {
> +        virReportError(VIR_ERR_OPERATION_INVALID,
> +                       "%s", _("domain is not running"));
> +        return ret;
> +    }
> +
> +    priv = vm->privateData;
> +
> +    qemuDomainObjEnterMonitor(driver, vm);
> +
> +    ret = qemuMonitorQueryDirtyRate(priv->mon, info);
> +    if (ret < 0) {
> +        virReportError(VIR_ERR_OPERATION_FAILED,
> +                       "%s", _("get vm's dirty rate failed."));

No, this is not correct, qemuMonitorQueryDirtyRate() will report an 
error if something goes wrong. And with pretty accurate error message. 
This overwrites it with more generic one.

> +    }
> +    if (qemuDomainObjExitMonitor(driver, vm) < 0)
> +        ret = -1;
> +
> +    return ret;
> +}
> diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
> index 0522b375c0..8baae512b7 100644
> --- a/src/qemu/qemu_migration.h
> +++ b/src/qemu/qemu_migration.h
> @@ -263,3 +263,8 @@ int
>   qemuDomainCalculateDirtyRate(virDomainPtr dom,
>                                virDomainObjPtr vm,
>                                long long sec);
> +
> +int
> +qemuDomainQueryDirtyRate(virDomainPtr dom,
> +                         virDomainObjPtr vm,
> +                         virDomainDirtyRateInfoPtr info);
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 06603b8691..2fa6879467 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -4781,3 +4781,15 @@ qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
>   
>       return qemuMonitorJSONCalculateDirtyRate(mon, sec);
>   }
> +
> +
> +int
> +qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
> +                          virDomainDirtyRateInfoPtr info)
> +{
> +    VIR_DEBUG("info=%p", info);
> +
> +    QEMU_CHECK_MONITOR(mon);
> +
> +    return qemuMonitorJSONQueryDirtyRate(mon, info);
> +}
> diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
> index afb97780cf..25105c3ad9 100644
> --- a/src/qemu/qemu_monitor.h
> +++ b/src/qemu/qemu_monitor.h
> @@ -1531,3 +1531,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
>   int
>   qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
>                                 long long sec);
> +
> +int
> +qemuMonitorQueryDirtyRate(qemuMonitorPtr mon,
> +                          virDomainDirtyRateInfoPtr info);
> diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
> index 65691522fb..1924c7229b 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -9657,3 +9657,25 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
>   
>       return 0;
>   }
> +
> +
> +int
> +qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
> +                              virDomainDirtyRateInfoPtr info)
> +{
> +    g_autoptr(virJSONValue) cmd = NULL;
> +    g_autoptr(virJSONValue) reply = NULL;
> +
> +    if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL)))
> +        return -1;
> +
> +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
> +        return -1;
> +
> +    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
> +        return -1;
> +
> +    /* TODO: extract dirtyrate data from reply and store in virDomainDirtyRateInfoPtr */

How about instead of noting down to finish this, squash 5/7 here? It's 
not like somebody could backport only this or that patch.

> +    info->status = 0;
> +    return 0;
> +}
> diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
> index c9556fc19a..487f2e6e58 100644
> --- a/src/qemu/qemu_monitor_json.h
> +++ b/src/qemu/qemu_monitor_json.h
> @@ -714,3 +714,7 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
>   int
>   qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
>                                     long long sec);
> +
> +int
> +qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon,
> +                              virDomainDirtyRateInfoPtr info);
> 

I think the following should be squashed in:

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 5a253bbe2f..c0df2589da 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5864,29 +5864,21 @@ qemuDomainCalculateDirtyRate(virQEMUDriverPtr 
driver,


  int
-qemuDomainQueryDirtyRate(virDomainPtr dom,
+qemuDomainQueryDirtyRate(virQEMUDriverPtr driver,
                           virDomainObjPtr vm,
                           virDomainDirtyRateInfoPtr info)
  {
-    virQEMUDriverPtr driver = dom->conn->privateData;
-    qemuDomainObjPrivatePtr priv;
-    int ret = -1;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    int ret;

      if (!virDomainObjIsActive(vm)) {
          virReportError(VIR_ERR_OPERATION_INVALID,
                         "%s", _("domain is not running"));
-        return ret;
+        return -1;
      }

-    priv = vm->privateData;
-
      qemuDomainObjEnterMonitor(driver, vm);
-
      ret = qemuMonitorQueryDirtyRate(priv->mon, info);
-    if (ret < 0) {
-        virReportError(VIR_ERR_OPERATION_FAILED,
-                       "%s", _("get vm's dirty rate failed."));
-    }
      if (qemuDomainObjExitMonitor(driver, vm) < 0)
          ret = -1;

diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index f03bbaa3d9..c4be340ecb 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -265,6 +265,6 @@ qemuDomainCalculateDirtyRate(virQEMUDriverPtr driver,
                               long long sec);

  int
-qemuDomainQueryDirtyRate(virDomainPtr dom,
+qemuDomainQueryDirtyRate(virQEMUDriverPtr driver,
                           virDomainObjPtr vm,
                           virDomainDirtyRateInfoPtr info);


Michal