On 2/16/22 01:28, huangy81@xxxxxxxxxxxxxxx wrote:
> From: Hyman Huang(黄勇) <huangy81@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> ---
> 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.
Michal
