在 2022/2/18 0:19, Michal Prívozník 写道:
On 2/16/22 01:28, huangy81@chinatelecom.cn wrote:
From: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
For any virTypedParameter API normal practice is to use a string to expose the data, not the rather enum integer value.
So let's drop the virDomainDirtyRateStatus in public header file and introduce internal enum def qemuMonitorDirtyRateStatus to describe the dirty page rate calculation status.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn> --- include/libvirt/libvirt-domain.h | 18 ------------------ src/libvirt-domain.c | 4 ++-- src/qemu/qemu_driver.c | 9 ++++++--- src/qemu/qemu_monitor.h | 18 ++++++++++++++++-- src/qemu/qemu_monitor_json.c | 4 ++-- 5 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 8c16598..bf4746a 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5241,24 +5241,6 @@ int virDomainGetMessages(virDomainPtr domain, char ***msgs, unsigned int flags);
-/** - * virDomainDirtyRateStatus: - * - * Details on the cause of a dirty rate calculation status. - */ -typedef enum { - VIR_DOMAIN_DIRTYRATE_UNSTARTED = 0, /* the dirtyrate calculation has - not been started */ - VIR_DOMAIN_DIRTYRATE_MEASURING = 1, /* the dirtyrate calculation is - measuring */ - VIR_DOMAIN_DIRTYRATE_MEASURED = 2, /* the dirtyrate calculation is - completed */ - -# ifdef VIR_ENUM_SENTINELS - VIR_DOMAIN_DIRTYRATE_LAST -# endif -} virDomainDirtyRateStatus; - int virDomainStartDirtyRateCalc(virDomainPtr domain, int seconds, unsigned int flags); diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index b8a6f10..f24d072 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -11947,8 +11947,8 @@ virConnectGetDomainCapabilities(virConnectPtr conn, * this format: * * "dirtyrate.calc_status" - the status of last memory dirty rate calculation, - * returned as int from virDomainDirtyRateStatus - * enum. + * either of these 3 'unstarted,measuring,measured' + * values returned. * "dirtyrate.calc_start_time" - the start time of last memory dirty rate * calculation as long long. * "dirtyrate.calc_period" - the period of last memory dirty rate calculation diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f262020..a22646c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18525,6 +18525,8 @@ qemuDomainGetStatsDirtyRateMon(virQEMUDriver *driver, return ret; }
+VIR_ENUM_DECL(qemuMonitorDirtyRateStatus); + static int qemuDomainGetStatsDirtyRate(virQEMUDriver *driver, virDomainObj *dom, @@ -18539,8 +18541,9 @@ qemuDomainGetStatsDirtyRate(virQEMUDriver *driver, if (qemuDomainGetStatsDirtyRateMon(driver, dom, &info) < 0) return -1;
- if (virTypedParamListAddInt(params, info.status, - "dirtyrate.calc_status") < 0) + if (virTypedParamListAddString(params, + qemuMonitorDirtyRateStatusTypeToString(info.status), + "dirtyrate.calc_status") < 0)
I worry that this change is not backwards compatible. I mean, what if I had an app that fetches dirtyrate.calc_status, expecting it to be an int?
In some languages this may be less of a problem (e.g. in python) but still, I would have to make changes to my app only because of this patch.
Indeed, there exists such scenario, so two policy can be choosed: 1. just drop the commit and ignore 2. add a new field like "dirtyrate.calc_ret_status" to reprensent these. which do you prefer?
Michal
-- Best regard Hyman Huang(黄勇)