Re: [PATCH v4 3/7] migration/dirtyrate: Implement qemuDomainCalculateDirtyRate

Michal Privoznik <mprivozn@xxxxxxxxxx> · Tue, 10 Nov 2020 21:11:43 +0100

On 11/7/20 10:41 AM, Hao Wang wrote:
Implement qemuDomainCalculateDirtyRate which calculates domain's memory
dirty rate calling qmp "calc-dirty-rate".

Signed-off-by: Hao Wang <wanghao232@xxxxxxxxxx>
Signed-off-by: Zhou Yimin <zhouyimin@xxxxxxxxxx>
Reviewed-by: Chuan Zheng <zhengchuan@xxxxxxxxxx>
---
  src/qemu/qemu_migration.c    | 28 ++++++++++++++++++++++++++++
  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, 75 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 6f764b0c73..8029e24415 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5836,3 +5836,31 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,
      virHashFree(blockinfo);
      return 0;
  }
+
+
+int
+qemuDomainCalculateDirtyRate(virDomainPtr dom,

The caller (implemented later in the series) has pointer to the driver. 
Might as well pass it here. Alternativelly, there is a piggy back 
pointer stored inside @vm's private data: QEMU_DOMAIN_PRIVATE(vm)->driver

+                             virDomainObjPtr vm,
+                             long long sec)
+{
+    virQEMUDriverPtr driver = dom->conn->privateData;
+    qemuDomainObjPrivatePtr priv;
+    int ret = -1;
+
+    if (!virDomainObjIsActive(vm)) {
+        virReportError(VIR_ERR_OPERATION_INVALID,
+                       "%s", _("domain is not running"));
+        return ret;
+    }
+
+    priv = vm->privateData;

It's okay to initialize priv when declaring it.

+
+    VIR_DEBUG("Calculate dirty rate during %lld seconds", sec);
+    qemuDomainObjEnterMonitor(driver, vm);
+
+    ret = qemuMonitorCalculateDirtyRate(priv->mon, sec);
+    if (qemuDomainObjExitMonitor(driver, vm) < 0)
+        ret = -1;
+
+    return ret;
+}
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index fd9eb7cab0..0522b375c0 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -258,3 +258,8 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,
                                   virDomainObjPtr vm,
                                   qemuDomainAsyncJob asyncJob,
                                   qemuDomainJobInfoPtr jobInfo);
+
+int
+qemuDomainCalculateDirtyRate(virDomainPtr dom,
+                             virDomainObjPtr vm,
+                             long long sec);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 5481bd99a0..06603b8691 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4769,3 +4769,15 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
      return qemuMonitorJSONTransactionBackup(actions, device, jobname, target,
                                              bitmap, syncmode);
  }
+
+
+int
+qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
+                              long long sec)
+{
+    VIR_DEBUG("seconds=%lld", sec);
+
+    QEMU_CHECK_MONITOR(mon);
+
+    return qemuMonitorJSONCalculateDirtyRate(mon, sec);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 54fbb41ef7..afb97780cf 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1527,3 +1527,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,
                               const char *target,
                               const char *bitmap,
                               qemuMonitorTransactionBackupSyncMode syncmode);
+
+int
+qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,
+                              long long sec);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 843a555952..65691522fb 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9635,3 +9635,25 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
      return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"),
                                    migratable);
  }
+
+
+int
+qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
+                                  long long sec)
+{
+    g_autoptr(virJSONValue) cmd = NULL;
+    g_autoptr(virJSONValue) reply = NULL;
+
+    if (!(cmd = qemuMonitorJSONMakeCommand("calc-dirty-rate",
+                                           "I:calc-time", (long)sec,

I'm not convinced on this typecast. qemuMonitorJSONMakeCommand() will 
under the hood pop long long. But anyway, @sec shouldn't be long long in 
the first place.

+                                           NULL)))
+        return -1;
+
+    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
+        return -1;
+
+    if (qemuMonitorJSONCheckError(cmd, reply) < 0)
+        return -1;
+
+    return 0;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 38e23ef3c5..c9556fc19a 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -710,3 +710,7 @@ int qemuMonitorJSONSetDBusVMStateIdList(qemuMonitorPtr mon,
  int
  qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,
                                  bool *migratable);
+
+int
+qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
+                                  long long sec);


I think the following should be squashed in:

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index c89ee39831..b468c68d2d 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -5839,25 +5839,22 @@ 
qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver,


 int
-qemuDomainCalculateDirtyRate(virDomainPtr dom,
+qemuDomainCalculateDirtyRate(virQEMUDriverPtr driver,
                              virDomainObjPtr vm,
                              long long sec)
 {
-    virQEMUDriverPtr driver = dom->conn->privateData;
-    qemuDomainObjPrivatePtr priv;
-    int ret = -1;
+    qemuDomainObjPrivatePtr priv = vm->privateData;
+    int ret;
+
+    VIR_DEBUG("Calculate dirty rate during %lld seconds", sec);

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

-    priv = vm->privateData;
-
-    VIR_DEBUG("Calculate dirty rate during %lld seconds", sec);
     qemuDomainObjEnterMonitor(driver, vm);
-
     ret = qemuMonitorCalculateDirtyRate(priv->mon, sec);
     if (qemuDomainObjExitMonitor(driver, vm) < 0)
         ret = -1;
diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h
index 0522b375c0..ee5f39655e 100644
--- a/src/qemu/qemu_migration.h
+++ b/src/qemu/qemu_migration.h
@@ -260,6 +260,6 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr 
driver,
                                  qemuDomainJobInfoPtr jobInfo);

 int
-qemuDomainCalculateDirtyRate(virDomainPtr dom,
+qemuDomainCalculateDirtyRate(virQEMUDriverPtr driver,
                              virDomainObjPtr vm,
                              long long sec);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 53c2a6521e..1f9d65b235 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -9645,7 +9645,7 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,
     g_autoptr(virJSONValue) reply = NULL;

     if (!(cmd = qemuMonitorJSONMakeCommand("calc-dirty-rate",
-                                           "I:calc-time", (long)sec,
+                                           "I:calc-time", sec,
                                            NULL)))
         return -1;


Michal







