Great thanks for these helpful suggestions. I'll introduce them into my next version. On 2020/11/11 4:11, Michal Privoznik wrote:
On 11/7/20 10:41 AM, Hao Wang wrote:
Introduce DomainGetDirtyRateInfo API for domain's memory dirty rate calculation and query.
Signed-off-by: Hao Wang <wanghao232@huawei.com> Signed-off-by: Zhou Yimin <zhouyimin@huawei.com> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com> --- include/libvirt/libvirt-domain.h | 5 ++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 46 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 ++++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 21 ++++++++++++++- 6 files changed, 84 insertions(+), 1 deletion(-)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 145f517068..b950736b67 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5120,4 +5120,9 @@ struct _virDomainDirtyRateInfo { typedef virDomainDirtyRateInfo *virDomainDirtyRateInfoPtr; +int virDomainGetDirtyRateInfo(virDomainPtr domain, + virDomainDirtyRateInfoPtr info, + long long sec, + unsigned int flags); + #endif /* LIBVIRT_DOMAIN_H */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index bce023017d..a77c29de54 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1387,6 +1387,12 @@ typedef char * (*virDrvDomainBackupGetXMLDesc)(virDomainPtr domain, unsigned int flags); +typedef int +(*virDrvDomainGetDirtyRateInfo)(virDomainPtr domain, + virDomainDirtyRateInfoPtr info, + long long sec, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; typedef virHypervisorDriver *virHypervisorDriverPtr; @@ -1650,4 +1656,5 @@ struct _virHypervisorDriver { virDrvDomainAgentSetResponseTimeout domainAgentSetResponseTimeout; virDrvDomainBackupBegin domainBackupBegin; virDrvDomainBackupGetXMLDesc domainBackupGetXMLDesc; + virDrvDomainGetDirtyRateInfo domainGetDirtyRateInfo; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 3c5f55176a..ce3c40edf8 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12758,3 +12758,49 @@ virDomainBackupGetXMLDesc(virDomainPtr domain, virDispatchError(conn); return NULL; } + + +/** + * virDomainGetDirtyRateInfo: + * @domain: a domain object. + * @info: return value of current domain's memory dirty rate info. + * @sec: show dirty rate within specified seconds. + * @flags: the flags of getdirtyrate action -- calculate and/or query.
What are the flags? Which enum should I look at?
+ * + * Get the current domain's memory dirty rate (in MB/s).
Can you expand on this a bit? Look at description to other APIs for inspiration.
+ * + * Returns 0 in case of success, -1 otherwise. + */ +int +virDomainGetDirtyRateInfo(virDomainPtr domain, + virDomainDirtyRateInfoPtr info, + long long sec,
Do we really need long long? That's more than 2.9*10^11 years. I don't think any virtual machine will ever run for so long (considering that the age of the universe is ~1.38*10^10 years)
+ unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);
@flags should also be logged.
+ + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckNonNullArgGoto(info, error); + virCheckReadOnlyGoto(conn->flags, error); + + if (info) + memset(info, 0, sizeof(*info));
@info was checked for NULL just two lines above.
+ + if (conn->driver->domainGetDirtyRateInfo) { + if (conn->driver->domainGetDirtyRateInfo(domain, info, sec, flags) < 0) + goto error; + VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec);
This is not very useful debug log.
+ return 0; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 539d2e3943..11864f48b1 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -873,4 +873,9 @@ LIBVIRT_6.0.0 { virDomainBackupGetXMLDesc; } LIBVIRT_5.10.0; +LIBVIRT_6.9.0 { + global: + virDomainGetDirtyRateInfo; +} LIBVIRT_6.0.0; +
Unfortunately, this missed 6.9.0 so this must be 6.10.0 instead.
# .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 9cd2fd36ae..325968a22b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8458,6 +8458,7 @@ static virHypervisorDriver hypervisor_driver = { .domainAgentSetResponseTimeout = remoteDomainAgentSetResponseTimeout, /* 5.10.0 */ .domainBackupBegin = remoteDomainBackupBegin, /* 6.0.0 */ .domainBackupGetXMLDesc = remoteDomainBackupGetXMLDesc, /* 6.0.0 */ + .domainGetDirtyRateInfo = remoteDomainGetDirtyRateInfo, /* 6.9.0 */
Same here.
}; static virNetworkDriver network_driver = {
I think the following should be squashed in:
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index e8bd890b29..594d7a2790 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5096,28 +5096,18 @@ int virDomainBackupBegin(virDomainPtr domain, char *virDomainBackupGetXMLDesc(virDomainPtr domain, unsigned int flags);
-/** - * virDomainDirtyRateFlags: - * - * Details on the flags used by getdirtyrate api. - */ - -typedef enum { - VIR_DOMAIN_DIRTYRATE_CALC = 1 << 0, /* calculate domain's dirtyrate */ - VIR_DOMAIN_DIRTYRATE_QUERY = 1 << 1, /* query domain's dirtyrate */ -} virDomainDirtyRateFlags; - /** * virDomainDirtyRateStatus: * - * Details on the cause of a dirtyrate calculation status. + * 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_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 */ + completed */
# ifdef VIR_ENUM_SENTINELS VIR_DOMAIN_DIRTYRATE_LAST @@ -5133,12 +5123,23 @@ completed */ typedef struct _virDomainDirtyRateInfo virDomainDirtyRateInfo; typedef virDomainDirtyRateInfo *virDomainDirtyRateInfoPtr; struct _virDomainDirtyRateInfo { - int status; /* the status of dirty rate calculation */ + int status; /* the status of dirty rate calculation, one of + virDomainDirtyRateStatus */ long long dirtyRate; /* the dirty rate in MB/s */ long long startTime; /* the start time of dirty rate calculation */ long long calcTime; /* the period of dirty rate calculation */ };
+/** + * virDomainDirtyRateFlags: + * + * Details on the flags used by getdirtyrate api. + */ +typedef enum { + VIR_DOMAIN_DIRTYRATE_CALC = 1 << 0, /* calculate domain's dirty rate */ + VIR_DOMAIN_DIRTYRATE_QUERY = 1 << 1, /* query domain's dirty rate */ +} virDomainGetDirtyRateInfoFlags; + int virDomainGetDirtyRateInfo(virDomainPtr domain, virDomainDirtyRateInfoPtr info, long long sec, diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index ce3c40edf8..75029b0a4b 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -12762,10 +12762,10 @@ virDomainBackupGetXMLDesc(virDomainPtr domain,
/** * virDomainGetDirtyRateInfo: - * @domain: a domain object. - * @info: return value of current domain's memory dirty rate info. - * @sec: show dirty rate within specified seconds. - * @flags: the flags of getdirtyrate action -- calculate and/or query. + * @domain: a domain object + * @info: pointer to current domain's memory dirty rate info + * @sec: show dirty rate within specified seconds + * @flags: extra flags; binary-OR of virDomainGetDirtyRateInfoFlags * * Get the current domain's memory dirty rate (in MB/s). * @@ -12779,7 +12779,8 @@ virDomainGetDirtyRateInfo(virDomainPtr domain, { virConnectPtr conn;
- VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec); + VIR_DOMAIN_DEBUG(domain, "info=%p, sec=%lld, flags=0x%x", + info, sec, flags);
virResetLastError();
@@ -12789,14 +12790,14 @@ virDomainGetDirtyRateInfo(virDomainPtr domain, virCheckNonNullArgGoto(info, error); virCheckReadOnlyGoto(conn->flags, error);
- if (info) - memset(info, 0, sizeof(*info)); + memset(info, 0, sizeof(*info));
if (conn->driver->domainGetDirtyRateInfo) { - if (conn->driver->domainGetDirtyRateInfo(domain, info, sec, flags) < 0) + int ret; + ret = conn->driver->domainGetDirtyRateInfo(domain, info, sec, flags); + if (ret < 0) goto error; - VIR_DOMAIN_DEBUG(domain, "info = %p, seconds=%lld", info, sec); - return 0; + return ret; }
virReportUnsupportedError();
Michal
.