On Tue, Nov 10, 2020 at 21:12:33 +0100, Michal Privoznik wrote:

On 11/7/20 10:41 AM, Hao Wang wrote:

...

Introduce virDomainDirtyRateInfo structure used for domain's memory dirty rate query.

Signed-off-by: Hao Wang <wanghao232@huawei.com> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com> --- include/libvirt/libvirt-domain.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index b3310729bf..145f517068 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5096,4 +5096,28 @@ int virDomainBackupBegin(virDomainPtr domain, char *virDomainBackupGetXMLDesc(virDomainPtr domain, unsigned int flags); +/** + * virDomainDirtyRateInfo: + * + * a virDomainDirtyRateInfo is a structure filled by virDomainGetDirtyRate() and + * extracting dirty rate infomation for a given active Domain. + */ + +typedef struct _virDomainDirtyRateInfo virDomainDirtyRateInfo; + +struct _virDomainDirtyRateInfo { + int status; /* the status of dirtyrate calculation */ + long long dirtyRate; /* the dirtyrate in MB/s */ + long long startTime; /* the start time of dirtyrate calculation */ + long long calcTime; /* the period of dirtyrate calculation */ +}; + +/** + * virDomainDirtyRateInfoPtr: + * + * a virDomainDirtyRateInfoPtr is a pointer to a virDomainDirtyRateInfo structure. + */ + +typedef virDomainDirtyRateInfo *virDomainDirtyRateInfoPtr; + #endif /* LIBVIRT_DOMAIN_H */

This should be squashed with 2/7 and bits of 5/7 and 6/7 which modify this file. The key here is that public API should go into one patch, driver implementation into other - it's easier to backport patches this way.

Additionally I'm not sure how likely is that the data will be extened at some point in the future, but using a struct is not extensible. I'd suggest that this gets reported in the domain stats API

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

.

On 11/7/20 10:41 AM, Hao Wang wrote:

Implement qemuDomainCalculateDirtyRate which calculates domain's memory dirty rate calling qmp "calc-dirty-rate".

Signed-off-by: Hao Wang <wanghao232@huawei.com> Signed-off-by: Zhou Yimin <zhouyimin@huawei.com> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com> --- src/qemu/qemu_migration.c | 28 ++++++++++++++++++++++++++++ src/qemu/qemu_migration.h | 5 +++++ src/qemu/qemu_monitor.c | 12 ++++++++++++ src/qemu/qemu_monitor.h | 4 ++++ src/qemu/qemu_monitor_json.c | 22 ++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 ++++ 6 files changed, 75 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 6f764b0c73..8029e24415 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5836,3 +5836,31 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver, virHashFree(blockinfo); return 0; } + + +int +qemuDomainCalculateDirtyRate(virDomainPtr dom,

The caller (implemented later in the series) has pointer to the driver. Might as well pass it here. Alternativelly, there is a piggy back pointer stored inside @vm's private data: QEMU_DOMAIN_PRIVATE(vm)->driver

+ virDomainObjPtr vm, + long long sec) +{ + virQEMUDriverPtr driver = dom->conn->privateData; + qemuDomainObjPrivatePtr priv; + int ret = -1; + + if (!virDomainObjIsActive(vm)) { + virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); + return ret; + } + + priv = vm->privateData;

It's okay to initialize priv when declaring it.

+ + VIR_DEBUG("Calculate dirty rate during %lld seconds", sec); + qemuDomainObjEnterMonitor(driver, vm); + + ret = qemuMonitorCalculateDirtyRate(priv->mon, sec); + if (qemuDomainObjExitMonitor(driver, vm) < 0) + ret = -1; + + return ret; +} diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index fd9eb7cab0..0522b375c0 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -258,3 +258,8 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver, virDomainObjPtr vm, qemuDomainAsyncJob asyncJob, qemuDomainJobInfoPtr jobInfo); + +int +qemuDomainCalculateDirtyRate(virDomainPtr dom, + virDomainObjPtr vm, + long long sec); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 5481bd99a0..06603b8691 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4769,3 +4769,15 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions, return qemuMonitorJSONTransactionBackup(actions, device, jobname, target, bitmap, syncmode); } + + +int +qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon, + long long sec) +{ + VIR_DEBUG("seconds=%lld", sec); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONCalculateDirtyRate(mon, sec); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 54fbb41ef7..afb97780cf 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1527,3 +1527,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions, const char *target, const char *bitmap, qemuMonitorTransactionBackupSyncMode syncmode); + +int +qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon, + long long sec); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 843a555952..65691522fb 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9635,3 +9635,25 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon, return virJSONValueGetBoolean(virJSONValueObjectGet(reply, "return"), migratable); } + + +int +qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon, + long long sec) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("calc-dirty-rate", + "I:calc-time", (long)sec,

I'm not convinced on this typecast. qemuMonitorJSONMakeCommand() will under the hood pop long long. But anyway, @sec shouldn't be long long in the first place.

+ NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 38e23ef3c5..c9556fc19a 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -710,3 +710,7 @@ int qemuMonitorJSONSetDBusVMStateIdList(qemuMonitorPtr mon, int qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon, bool *migratable); + +int +qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon, + long long sec);

I think the following should be squashed in: diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index c89ee39831..b468c68d2d 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5839,25 +5839,22 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver, int -qemuDomainCalculateDirtyRate(virDomainPtr dom, +qemuDomainCalculateDirtyRate(virQEMUDriverPtr driver, virDomainObjPtr vm, long long sec) { - virQEMUDriverPtr driver = dom->conn->privateData; - qemuDomainObjPrivatePtr priv; - int ret = -1; + qemuDomainObjPrivatePtr priv = vm->privateData; + int ret; + + VIR_DEBUG("Calculate dirty rate during %lld seconds", sec); if (!virDomainObjIsActive(vm)) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain is not running")); - return ret; + return -1; } - priv = vm->privateData; - - VIR_DEBUG("Calculate dirty rate during %lld seconds", sec); qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorCalculateDirtyRate(priv->mon, sec); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -1; diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 0522b375c0..ee5f39655e 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -260,6 +260,6 @@ qemuMigrationSrcFetchMirrorStats(virQEMUDriverPtr driver, qemuDomainJobInfoPtr jobInfo); int -qemuDomainCalculateDirtyRate(virDomainPtr dom, +qemuDomainCalculateDirtyRate(virQEMUDriverPtr driver, virDomainObjPtr vm, long long sec); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 53c2a6521e..1f9d65b235 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9645,7 +9645,7 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon, g_autoptr(virJSONValue) reply = NULL; if (!(cmd = qemuMonitorJSONMakeCommand("calc-dirty-rate", - "I:calc-time", (long)sec, + "I:calc-time", sec, NULL))) return -1; Michal

Thanks for these reviews too. I'll apply all of them. On 2020/11/11 4:11, Michal Privoznik wrote:

On 11/7/20 10:41 AM, Hao Wang wrote:

...

Implement qemuDomainQueryDirtyRate which query domain's memory dirty rate calling qmp "query-dirty-rate".

Signed-off-by: Hao Wang <wanghao232@huawei.com> Signed-off-by: Zhou Yimin <zhouyimin@huawei.com> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com> ---   src/qemu/qemu_migration.c    | 31 +++++++++++++++++++++++++++++++   src/qemu/qemu_migration.h    |  5 +++++   src/qemu/qemu_monitor.c      | 12 ++++++++++++   src/qemu/qemu_monitor.h      |  4 ++++   src/qemu/qemu_monitor_json.c | 22 ++++++++++++++++++++++   src/qemu/qemu_monitor_json.h |  4 ++++   6 files changed, 78 insertions(+)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 8029e24415..3d07ba3ac4 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5864,3 +5864,34 @@ qemuDomainCalculateDirtyRate(virDomainPtr dom,         return ret;   } + + +int +qemuDomainQueryDirtyRate(virDomainPtr dom, +                         virDomainObjPtr vm, +                         virDomainDirtyRateInfoPtr info) +{ +    virQEMUDriverPtr driver = dom->conn->privateData;

Again, driver is accessible from the caller, just pass it directly.

...

+    qemuDomainObjPrivatePtr priv; +    int ret = -1; + +    if (!virDomainObjIsActive(vm)) { +        virReportError(VIR_ERR_OPERATION_INVALID, +                       "%s", _("domain is not running")); +        return ret; +    } + +    priv = vm->privateData; + +    qemuDomainObjEnterMonitor(driver, vm); + +    ret = qemuMonitorQueryDirtyRate(priv->mon, info); +    if (ret < 0) { +        virReportError(VIR_ERR_OPERATION_FAILED, +                       "%s", _("get vm's dirty rate failed."));

No, this is not correct, qemuMonitorQueryDirtyRate() will report an error if something goes wrong. And with pretty accurate error message. This overwrites it with more generic one.

...

+    } +    if (qemuDomainObjExitMonitor(driver, vm) < 0) +        ret = -1; + +    return ret; +} diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index 0522b375c0..8baae512b7 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -263,3 +263,8 @@ int   qemuDomainCalculateDirtyRate(virDomainPtr dom,                                virDomainObjPtr vm,                                long long sec); + +int +qemuDomainQueryDirtyRate(virDomainPtr dom, +                         virDomainObjPtr vm, +                         virDomainDirtyRateInfoPtr info); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 06603b8691..2fa6879467 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4781,3 +4781,15 @@ qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,         return qemuMonitorJSONCalculateDirtyRate(mon, sec);   } + + +int +qemuMonitorQueryDirtyRate(qemuMonitorPtr mon, +                          virDomainDirtyRateInfoPtr info) +{ +    VIR_DEBUG("info=%p", info); + +    QEMU_CHECK_MONITOR(mon); + +    return qemuMonitorJSONQueryDirtyRate(mon, info); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index afb97780cf..25105c3ad9 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1531,3 +1531,7 @@ qemuMonitorTransactionBackup(virJSONValuePtr actions,   int   qemuMonitorCalculateDirtyRate(qemuMonitorPtr mon,                                 long long sec); + +int +qemuMonitorQueryDirtyRate(qemuMonitorPtr mon, +                          virDomainDirtyRateInfoPtr info); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 65691522fb..1924c7229b 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9657,3 +9657,25 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,         return 0;   } + + +int +qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon, +                              virDomainDirtyRateInfoPtr info) +{ +    g_autoptr(virJSONValue) cmd = NULL; +    g_autoptr(virJSONValue) reply = NULL; + +    if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL))) +        return -1; + +    if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) +        return -1; + +    if (qemuMonitorJSONCheckError(cmd, reply) < 0) +        return -1; + +    /* TODO: extract dirtyrate data from reply and store in virDomainDirtyRateInfoPtr */

How about instead of noting down to finish this, squash 5/7 here? It's not like somebody could backport only this or that patch.

...

+    info->status = 0; +    return 0; +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index c9556fc19a..487f2e6e58 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -714,3 +714,7 @@ qemuMonitorJSONGetCPUMigratable(qemuMonitorPtr mon,   int   qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon,                                     long long sec); + +int +qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon, +                              virDomainDirtyRateInfoPtr info);

I think the following should be squashed in:

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5a253bbe2f..c0df2589da 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -5864,29 +5864,21 @@ qemuDomainCalculateDirtyRate(virQEMUDriverPtr driver,

 int -qemuDomainQueryDirtyRate(virDomainPtr dom, +qemuDomainQueryDirtyRate(virQEMUDriverPtr driver,                           virDomainObjPtr vm,                           virDomainDirtyRateInfoPtr info)  { -    virQEMUDriverPtr driver = dom->conn->privateData; -    qemuDomainObjPrivatePtr priv; -    int ret = -1; +    qemuDomainObjPrivatePtr priv = vm->privateData; +    int ret;

     if (!virDomainObjIsActive(vm)) {          virReportError(VIR_ERR_OPERATION_INVALID,                         "%s", _("domain is not running")); -        return ret; +        return -1;      }

-    priv = vm->privateData; -      qemuDomainObjEnterMonitor(driver, vm); -      ret = qemuMonitorQueryDirtyRate(priv->mon, info); -    if (ret < 0) { -        virReportError(VIR_ERR_OPERATION_FAILED, -                       "%s", _("get vm's dirty rate failed.")); -    }      if (qemuDomainObjExitMonitor(driver, vm) < 0)          ret = -1;

diff --git a/src/qemu/qemu_migration.h b/src/qemu/qemu_migration.h index f03bbaa3d9..c4be340ecb 100644 --- a/src/qemu/qemu_migration.h +++ b/src/qemu/qemu_migration.h @@ -265,6 +265,6 @@ qemuDomainCalculateDirtyRate(virQEMUDriverPtr driver,                               long long sec);

 int -qemuDomainQueryDirtyRate(virDomainPtr dom, +qemuDomainQueryDirtyRate(virQEMUDriverPtr driver,                           virDomainObjPtr vm,                           virDomainDirtyRateInfoPtr info);

Michal

.

On 11/7/20 10:41 AM, Hao Wang wrote:

Implement qemuMonitorJSONExtractDirtyRateInfo to deal with the return from qmp "query-dirty-rate", and store them in virDomainDirtyRateInfo.

Signed-off-by: Hao Wang <wanghao232@huawei.com> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com> --- include/libvirt/libvirt-domain.h | 17 +++++++++ src/qemu/qemu_monitor_json.c | 59 ++++++++++++++++++++++++++++++-- 2 files changed, 73 insertions(+), 3 deletions(-)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index b950736b67..51d8685086 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5096,6 +5096,23 @@ int virDomainBackupBegin(virDomainPtr domain, char *virDomainBackupGetXMLDesc(virDomainPtr domain, unsigned int flags);

+/** + * virDomainDirtyRateStatus: + * + * Details on the cause of a dirtyrate 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; +

As advertised earlier, this doesn't belong into this commit really.

/** * virDomainDirtyRateInfo: * diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 1924c7229b..ca7d8d23c0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9659,12 +9659,61 @@ qemuMonitorJSONCalculateDirtyRate(qemuMonitorPtr mon, }

+VIR_ENUM_DECL(qemuDomainDirtyRateStatus); +VIR_ENUM_IMPL(qemuDomainDirtyRateStatus, + VIR_DOMAIN_DIRTYRATE_LAST, + "unstarted", + "measuring", + "measured"); + +static int +qemuMonitorJSONExtractDirtyRateInfo(virJSONValuePtr data, + virDomainDirtyRateInfoPtr info) +{ + const char *status; + int statusID; + + if (!(status = virJSONValueObjectGetString(data, "status"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-dirty-rate reply was missing 'status' data")); + return -1; + } + + if ((statusID = qemuDomainDirtyRateStatusTypeFromString(status)) < 0) { + return -1;

So if qemu sends us some other string, this fails silently.

+ } + info->status = statusID; + + if ((info->status == VIR_DOMAIN_DIRTYRATE_MEASURED) && + (virJSONValueObjectGetNumberLong(data, "dirty-rate", &(info->dirtyRate)) < 0)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-dirty-rate reply was missing 'dirty-rate' data")); + return -1; + } + + if (virJSONValueObjectGetNumberLong(data, "start-time", &(info->startTime)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-dirty-rate reply was missing 'start-time' data")); + return -1; + } + + if (virJSONValueObjectGetNumberLong(data, "calc-time", &(info->calcTime)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-dirty-rate reply was missing 'calc-time' data")); + return -1; + } + + return 0; +} + + int qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon, virDomainDirtyRateInfoPtr info) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; + virJSONValuePtr data = NULL;

if (!(cmd = qemuMonitorJSONMakeCommand("query-dirty-rate", NULL))) return -1; @@ -9675,7 +9724,11 @@ qemuMonitorJSONQueryDirtyRate(qemuMonitorPtr mon, if (qemuMonitorJSONCheckError(cmd, reply) < 0) return -1;

- /* TODO: extract dirtyrate data from reply and store in virDomainDirtyRateInfoPtr */ - info->status = 0; - return 0; + if (!(data = virJSONValueObjectGetObject(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-dirty-rate reply was missing 'return' data")); + return -1; + } + + return qemuMonitorJSONExtractDirtyRateInfo(data, info); }

I think the following should be squashed in: diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 78ad10bc24..4715b33254 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -9680,24 +9680,26 @@ qemuMonitorJSONExtractDirtyRateInfo(virJSONValuePtr data, } if ((statusID = qemuDomainDirtyRateStatusTypeFromString(status)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown dirty rate status: %s"), status); return -1; } info->status = statusID; if ((info->status == VIR_DOMAIN_DIRTYRATE_MEASURED) && - (virJSONValueObjectGetNumberLong(data, "dirty-rate", &(info->dirtyRate)) < 0)) { + (virJSONValueObjectGetNumberLong(data, "dirty-rate", &info->dirtyRate) < 0)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-dirty-rate reply was missing 'dirty-rate' data")); return -1; } - if (virJSONValueObjectGetNumberLong(data, "start-time", &(info->startTime)) < 0) { + if (virJSONValueObjectGetNumberLong(data, "start-time", &info->startTime) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-dirty-rate reply was missing 'start-time' data")); return -1; } - if (virJSONValueObjectGetNumberLong(data, "calc-time", &(info->calcTime)) < 0) { + if (virJSONValueObjectGetNumberLong(data, "calc-time", &info->calcTime) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-dirty-rate reply was missing 'calc-time' data")); return -1; Michal

Thanks for the reviews. On 2020/11/11 4:11, Michal Privoznik wrote:

On 11/7/20 10:41 AM, Hao Wang wrote:

...

Implement qemuDomainGetDirtyRateInfo: using flags to control behaviors -- calculate and/or query dirtyrate.

Signed-off-by: Hao Wang <wanghao232@huawei.com> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com> ---   include/libvirt/libvirt-domain.h | 11 ++++++   src/qemu/qemu_driver.c           | 68 ++++++++++++++++++++++++++++++++   2 files changed, 79 insertions(+)

diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 51d8685086..fc45f42dcf 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -5096,6 +5096,17 @@ 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:    *

Again, doesn't belong here.

...

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cb56fbbfcf..93d5a23630 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20121,6 +20121,73 @@ qemuDomainAgentSetResponseTimeout(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, +                           long long sec, +                           unsigned int flags) +{ +    virDomainObjPtr vm = NULL; +    virQEMUDriverPtr driver = dom->conn->privateData; +    int ret = -1; + +    if (!(vm = qemuDomainObjFromDomain(dom))) +        return ret; + +    if (virDomainGetDirtyRateInfoEnsureACL(dom->conn, vm->def) < 0) +        goto cleanup; + +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0) +        goto cleanup; + +    if (!qemuMigrationSrcIsAllowed(driver, vm, false, 0)) +        goto endjob; +

Why is this check needed? I don't understand it, can you please explain?

It's indeed not necessary. I'll remove it.

...

+    if (flags & VIR_DOMAIN_DIRTYRATE_CALC) { +        if (sec < MIN_DIRTYRATE_CALCULATION_PERIOD || sec > MAX_DIRTYRATE_CALCULATION_PERIOD) { +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, +                           _("seconds=%lld is invalid, please choose value within [1, 60]."), +                           sec); +            goto endjob; +        } + +        if (qemuDomainCalculateDirtyRate(dom, vm, sec) < 0) { +            virReportError(VIR_ERR_OPERATION_FAILED, "%s", +                           _("can't calculate domain's dirty rate"));

This overwrites a more accurate error reported by qemuDomainCalculateDirtyRate().

...

+            goto endjob; +        } +    } + +    if (flags & VIR_DOMAIN_DIRTYRATE_QUERY) { +        if (flags & VIR_DOMAIN_DIRTYRATE_CALC) { +            struct timespec ts = { .tv_sec = sec, .tv_nsec = 50 * 1000 * 1000ull }; + +            virObjectUnlock(vm); +            nanosleep(&ts, NULL); +            virObjectLock(vm);

At first I was afraid, I was petrified that this waits for 50 seconds, until I realized it's nanosleep(). Perhaps g_usleep(50*1000); would look better?

...

+        } + +        if (qemuDomainQueryDirtyRate(dom, vm, info) < 0) { +            virReportError(VIR_ERR_OPERATION_FAILED, "%s", +                           _("can't query domain's dirty rate"));

Again, error overwrite.

...

+            goto endjob; +        } +    }

So if no flag is specified then nothing happens? I know you handle that in virsh, but I think that logic should live here. And perhaps the public API description should document that no flags is equal to specifying both flags together.

...

+ +    ret = 0; + + endjob: +    qemuDomainObjEndJob(driver, vm); + + cleanup: +    virDomainObjEndAPI(&vm); +    return ret; +} + +   static virHypervisorDriver qemuHypervisorDriver = {       .name = QEMU_DRIVER_NAME,       .connectURIProbe = qemuConnectURIProbe, @@ -20360,6 +20427,7 @@ static virHypervisorDriver qemuHypervisorDriver = {       .domainAgentSetResponseTimeout = qemuDomainAgentSetResponseTimeout, /* 5.10.0 */       .domainBackupBegin = qemuDomainBackupBegin, /* 6.0.0 */       .domainBackupGetXMLDesc = qemuDomainBackupGetXMLDesc, /* 6.0.0 */ +    .domainGetDirtyRateInfo = qemuDomainGetDirtyRateInfo, /* 6.9.0 */

6.10.0 :-(

...

  };    

I think the following should be squashed in:

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4cbbe8dd84..4058fb487c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20130,12 +20130,12 @@ qemuDomainGetDirtyRateInfo(virDomainPtr dom,                             long long sec,                             unsigned int flags)  { -    virDomainObjPtr vm = NULL;      virQEMUDriverPtr driver = dom->conn->privateData; +    virDomainObjPtr vm = NULL;      int ret = -1;

     if (!(vm = qemuDomainObjFromDomain(dom))) -        return ret; +        return -1;

     if (virDomainGetDirtyRateInfoEnsureACL(dom->conn, vm->def) < 0)          goto cleanup; @@ -20147,18 +20147,18 @@ qemuDomainGetDirtyRateInfo(virDomainPtr dom,          goto endjob;

     if (flags & VIR_DOMAIN_DIRTYRATE_CALC) { -        if (sec < MIN_DIRTYRATE_CALCULATION_PERIOD || sec > MAX_DIRTYRATE_CALCULATION_PERIOD) { +        if (sec < MIN_DIRTYRATE_CALCULATION_PERIOD || +            sec > MAX_DIRTYRATE_CALCULATION_PERIOD) {              virReportError(VIR_ERR_CONFIG_UNSUPPORTED, -                           _("seconds=%lld is invalid, please choose value within [1, 60]."), -                           sec); +                           _("seconds=%lld is invalid, please choose value within [%d, %d]."), +                           sec, +                           MIN_DIRTYRATE_CALCULATION_PERIOD, +                           MAX_DIRTYRATE_CALCULATION_PERIOD);              goto endjob;          }

-        if (qemuDomainCalculateDirtyRate(dom, vm, sec) < 0) { -            virReportError(VIR_ERR_OPERATION_FAILED, "%s", -                           _("can't calculate domain's dirty rate")); +        if (qemuDomainCalculateDirtyRate(driver, vm, sec) < 0)              goto endjob; -        }      }

     if (flags & VIR_DOMAIN_DIRTYRATE_QUERY) { @@ -20170,11 +20170,8 @@ qemuDomainGetDirtyRateInfo(virDomainPtr dom,              virObjectLock(vm);          }

-        if (qemuDomainQueryDirtyRate(dom, vm, info) < 0) { -            virReportError(VIR_ERR_OPERATION_FAILED, "%s", -                           _("can't query domain's dirty rate")); +        if (qemuDomainQueryDirtyRate(driver, vm, info) < 0)              goto endjob; -        }      }

     ret = 0; @@ -20427,7 +20424,7 @@ static virHypervisorDriver qemuHypervisorDriver = {      .domainAgentSetResponseTimeout = qemuDomainAgentSetResponseTimeout, /* 5.10.0 */      .domainBackupBegin = qemuDomainBackupBegin, /* 6.0.0 */      .domainBackupGetXMLDesc = qemuDomainBackupGetXMLDesc, /* 6.0.0 */ -    .domainGetDirtyRateInfo = qemuDomainGetDirtyRateInfo, /* 6.9.0 */ +    .domainGetDirtyRateInfo = qemuDomainGetDirtyRateInfo, /* 6.10.0 */  };

Michal

.

On 11/7/20 10:41 AM, Hao Wang wrote:

Signed-off-by: Hao Wang <wanghao232@huawei.com> Signed-off-by: Zhou Yimin <zhouyimin@huawei.com> Reviewed-by: Chuan Zheng <zhengchuan@huawei.com> --- tools/virsh-domain.c | 112 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+)

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ef347585e8..52608e2c1b 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -14374,6 +14374,112 @@ cmdGuestInfo(vshControl *ctl, const vshCmd *cmd) return ret; }

+/* + * "querydirtyrate" command + */ +static const vshCmdInfo info_getdirtyrate[] = { + {.name = "help", + .data = N_("Get a vm's memory dirty rate") + }, + {.name = "desc", + .data = N_("Get memory dirty rate of a domain in order to decide" + " whether it's proper to be migrated out or not.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_getdirtyrate[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "seconds", + .type = VSH_OT_INT, + .help = N_("calculate memory dirty rate within specified seconds," + " a valid range of values is [1, 60], and would default to 1s.") + }, + {.name = "calculate", + .type = VSH_OT_BOOL, + .help = N_("calculate dirty rate only, can be used together with --query," + " either or both is expected, otherwise would default to both.") + }, + {.name = "query", + .type = VSH_OT_BOOL, + .help = N_("query dirty rate only, can be used together with --calculate," + " either or both is expected, otherwise would default to both.") + }, + {.name = NULL} +}; + +static bool +cmdGetDirtyRateInfo(vshControl *ctl, const vshCmd *cmd) +{ + virDomainPtr dom = NULL; + virDomainDirtyRateInfo info; + long long sec = 0; + const char *status = NULL; + unsigned int flags = 0; + int rc; + bool ret = false; + bool calc = vshCommandOptBool(cmd, "calculate"); + bool query = vshCommandOptBool(cmd, "query"); + + if (calc) + flags |= VIR_DOMAIN_DIRTYRATE_CALC; + if (query) + flags |= VIR_DOMAIN_DIRTYRATE_QUERY; + + /* if flag option is missing, default to both --calculate and --query */ + if (!calc && !query) + flags |= VIR_DOMAIN_DIRTYRATE_CALC | VIR_DOMAIN_DIRTYRATE_QUERY; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + rc = vshCommandOptLongLong(ctl, cmd, "seconds", &sec); + if (rc < 0) + goto done; + + /* if --seconds option is missing, default to 1s */ + if (!rc) + sec = 1; + + if (virDomainGetDirtyRateInfo(dom, &info, sec, flags) < 0) { + vshError(ctl, "%s", _("Get memory dirty-rate failed."));

This is redundant. virsh prints error reported by a public API on failure. All that's needed is for this function to return false.

+ goto done; + } + + if (flags & VIR_DOMAIN_DIRTYRATE_QUERY) { + switch (info.status) { + case VIR_DOMAIN_DIRTYRATE_UNSTARTED: + status = _("unstarted"); + break; + case VIR_DOMAIN_DIRTYRATE_MEASURING: + status = _("measuring"); + break; + case VIR_DOMAIN_DIRTYRATE_MEASURED: + status = _("measured"); + break; + default: + status = _("unknown"); + } + + vshPrint(ctl, _("status: %s\n"), status); + vshPrint(ctl, _("start time: %lld\n"), info.startTime); + vshPrint(ctl, _("calc time: %lld s\n"), info.calcTime);

How about using vshTable so that you don't have to align this by hand?

+ + if (info.status == VIR_DOMAIN_DIRTYRATE_MEASURED) + vshPrint(ctl, _("dirty rate: %lld MB/s\n"), info.dirtyRate); + else + vshPrint(ctl, _("dirty rate: the calculation is %s, please query results later\n"), + status); + } else { + vshPrint(ctl, _("Memory dirty rate is calculating, use --query option to display results.\n")); + } + + ret = true; + done: + virshDomainFree(dom); + return ret; +} + const vshCmdDef domManagementCmds[] = { {.name = "attach-device", .handler = cmdAttachDevice, @@ -15001,5 +15107,11 @@ const vshCmdDef domManagementCmds[] = { .info = info_guestinfo, .flags = 0 }, + {.name = "getdirtyrate", + .handler = cmdGetDirtyRateInfo, + .opts = opts_getdirtyrate, + .info = info_getdirtyrate, + .flags = 0 + }, {.name = NULL} };

Missing manpage: it lives under docs/manpages/virsh.rst Michal