6:11 a.m.
Thanks for these reviews too. I'll apply all of them.
On 2020/11/11 4:11, Michal Privoznik wrote:
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(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 | 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
.