Re: [PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate
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 >> Signed-off-by: Zhou Yimin >> Reviewed-by: Chuan Zheng >> --- >> 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
Re: [PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate
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 Signed-off-by: Zhou Yimin Reviewed-by: Chuan Zheng --- 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
[PATCH v4 4/7] migration/dirtyrate: Implement qemuDomainQueryDirtyRate
Implement qemuDomainQueryDirtyRate which query domain's memory dirty rate calling qmp "query-dirty-rate". Signed-off-by: Hao Wang Signed-off-by: Zhou Yimin Reviewed-by: Chuan Zheng --- 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; +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.")); +} +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 */ +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); -- 2.23.0