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(a)huawei.com>
> Signed-off-by: Zhou Yimin <zhouyimin(a)huawei.com>
> Reviewed-by: Chuan Zheng <zhengchuan(a)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
.