On 2/1/21 10:55 AM, Hao Wang wrote:
Implement qemuDomainGetDirtyRateInfo: using flags to control behaviors -- calculate and/or query dirtyrate. Default flag is both calculate and query.
Signed-off-by: Hao Wang <wanghao232@huawei.com> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com> --- src/qemu/qemu_driver.c | 67 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ed840a5c8d..2b9ce1c386 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20289,6 +20289,72 @@ qemuDomainAuthorizedSSHKeysSet(virDomainPtr dom, }
+#define MIN_DIRTYRATE_CALCULATION_PERIOD 1 /* supported min dirtyrate calc time: 1s */ +#define MAX_DIRTYRATE_CALCULATION_PERIOD 60 /* supported max dirtyrate calc time: 60s */ + +static int +qemuDomainGetDirtyRateInfo(virDomainPtr dom, + virDomainDirtyRateInfoPtr info, + int sec, + unsigned int flags) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + virDomainObjPtr vm = NULL; + int ret = -1;
Here should be virCheckFlags() call with all flags supported enumarated. The idea is that if a new flag is ever invented then instead of ignoring it silently an error is produced.
+ + if (!(vm = qemuDomainObjFromDomain(dom))) + return -1; + + if (virDomainGetDirtyRateInfoEnsureACL(dom->conn, vm->def) < 0) + goto cleanup; + + if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) + goto cleanup; + + /* flags is default to both calculate and query */ + if (flags == VIR_DOMAIN_DIRTYRATE_DEFAULT) + flags |= VIR_DOMAIN_DIRTYRATE_CALC | VIR_DOMAIN_DIRTYRATE_QUERY; + + /* calculating */ + if (flags & VIR_DOMAIN_DIRTYRATE_CALC) { + if (sec < MIN_DIRTYRATE_CALCULATION_PERIOD || + sec > MAX_DIRTYRATE_CALCULATION_PERIOD) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("seconds=%d is invalid, please choose value within [%d, %d]."), + sec, + MIN_DIRTYRATE_CALCULATION_PERIOD, + MAX_DIRTYRATE_CALCULATION_PERIOD); + goto endjob; + } + + if (qemuDomainCalculateDirtyRate(driver, vm, sec) < 0) + goto endjob; + } + + /* querying */ + if (flags & VIR_DOMAIN_DIRTYRATE_QUERY) { + /* wait sec and extra 50ms to let last calculation finish */ + if (flags & VIR_DOMAIN_DIRTYRATE_CALC) { + virObjectUnlock(vm); + g_usleep((sec * 1000 + 50) * 1000); + virObjectLock(vm);
I know this will probably change, but anyway - waiting X seconds is not good IMO. Also, it performs two operations at once. What if the calculation was successfully started but then querying fails? How can a caller distinguish this from a case where the calculation failed to start? Michal