6:03 a.m.
Great thanks for these helpful suggestions. I'll introduce them into my next version.
On 2020/11/11 4:11, Michal Privoznik wrote:
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(a)huawei.com>
> Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
> Reviewed-by: Chuan Zheng <zhengchuan(a)huawei.com>
> ---
> 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
.