[PATCH Libvirt 00/11] Support dirty page rate upper limit

QEMU introduced the dirty page rate limit feature in 7.1.0, see the details in the following link: https://lore.kernel.org/qemu- devel/cover.1656177590.git.huangy81@chinatelecom.cn/ So maybe it's the right time to enable this feature in libvirt and the upper user can play with it, expecting the upper app can use this feature to do a virtual CPU Qos or whatever else. Introduce the virsh API as follows: # virsh limit-dirty-page-rate <domain> [--rate <number>] [--vcpu <number>] [--cancel] Examples: To set the dirty page rate upper limit 60MB/s for all virtual CPUs in c81_node1, use: # virsh limit-dirty-page-rate c81_node1 --rate 60 Set dirty page rate limit 60(MB/s) on all virtual CPUs successfully To set the dirty page rate upper limit 35MB/s for virtual CPU 1 in c81_node1, use: # virsh limit-dirty-page-rate c81_node1 --rate 35 --vcpu 1 Set vcpu[1] dirty page rate upper limit 35(MB/s) successfully Specify the 'cancel' option to do the reverse, the optional option 'vcpu' is used to specify the CPU index to be set. To query the dirty page rate upper limit, use: # virsh vcpuinfo c81_node1 VCPU: 0 CPU: 14 State: running CPU time: 27.1s CPU Affinity: yyyyyyyyyyyyyyyy DirtyRate limit: 60 DirtyRate current: 0 VCPU: 1 CPU: 1 State: running CPU time: 25.1s CPU Affinity: yyyyyyyyyyyyyyyy DirtyRate limit: 35 DirtyRate current: 0 VCPU: 2 CPU: 7 State: running CPU time: 6.0s CPU Affinity: yyyyyyyyyyyyyyyy DirtyRate limit: 60 DirtyRate current: 0 VCPU: 3 CPU: 8 State: running CPU time: 3.5s CPU Affinity: yyyyyyyyyyyyyyyy DirtyRate limit: 60 DirtyRate current: 0 The patch set adds two new APIs to implement a dirty page rate limit: 1. virDomainSetVcpuDirtyLimit, which set virtual CPU dirty page rate limit. virsh command 'limit-dirty-page-rate' correspondingly. 2. virDomainCancelVcpuDirtyLimit, which cancel virtual CPU dirty page rate limit. 'cancel' option was introduced to 'limit-dirty-page-rate' to cancel the limit correspondingly. In addition, function 'qemuMonitorQueryVcpuDirtyLimit' was implemented to query the dirty page rate upper limit, the virsh command 'vcpuinfo' was extended. So that the user can query dirty page rate limit info via 'vcpuinfo'. This series makes the main modifications as follows: - introduce QEMU_CAPS_VCPU_DIRTY_LIMIT capability so that libvirt can probe before using dirty page rate upper limit feature. - implement virsh command 'limit-dirty-page-rate' to set/cancel dirty page rate upper limit. - extend 'vcpuinfo' API so that it can display dirty page rate upper limit. - document dirty page rate limit feature. Please review, and hoping the comments, thanks ! Yong Hyman Huang(黄勇) (11): qemu_capabilities: Introduce QEMU_CAPS_VCPU_DIRTY_LIMIT capability libvirt: Add virDomainSetVcpuDirtyLimit API qemu_driver: Implement qemuDomainSetVcpuDirtyLimit virsh: Introduce limit-dirty-page-rate api qemu_monitor: Implement qemuMonitorQueryVcpuDirtyLimit qemu_driver: Extend qemuDomainGetVcpus virsh: Extend vcpuinfo api libvirt: Add virDomainCancelVcpuDirtyLimit API qemu_driver: Implement qemuDomainCancelVcpuDirtyLimit virsh: Add cancel option of limit-dirty-page-rate api NEWS: Document limit dirty page rate APIs NEWS.rst | 16 ++ include/libvirt/libvirt-domain.h | 22 +++ src/driver-hypervisor.h | 13 ++ src/libvirt-domain.c | 106 ++++++++++++ src/libvirt_public.syms | 6 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c | 154 ++++++++++++++++++ src/qemu/qemu_monitor.c | 36 ++++ src/qemu/qemu_monitor.h | 26 +++ src/qemu/qemu_monitor_json.c | 150 +++++++++++++++++ src/qemu/qemu_monitor_json.h | 13 ++ src/remote/remote_daemon_dispatch.c | 2 + src/remote/remote_driver.c | 4 + src/remote/remote_protocol.x | 28 +++- src/remote_protocol-structs | 13 ++ .../qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + .../caps_7.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + .../caps_7.2.0_x86_64+hvf.xml | 1 + .../caps_7.2.0_x86_64.xml | 1 + .../caps_8.0.0_riscv64.xml | 1 + .../caps_8.0.0_x86_64.xml | 1 + .../qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + .../caps_8.1.0_x86_64.xml | 1 + tools/virsh-domain.c | 123 ++++++++++++++ 26 files changed, 723 insertions(+), 1 deletion(-) -- 2.38.5

From: Hyman Huang(黄勇) <yong.huang@smartx.com> Introduce virDomainSetVcpuDirtyLimit API to set upper limit of dirty page rate. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- include/libvirt/libvirt-domain.h | 16 ++++++++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 54 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 ++++++++- src/remote_protocol-structs | 7 +++++ 7 files changed, 104 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index a1902546bb..df7deffaa9 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6506,4 +6506,20 @@ int virDomainFDAssociate(virDomainPtr domain, int *fds, unsigned int flags); +/** + * virDomainDirtyLimitFlags: + * + * Since: 9.6.0 + */ +typedef enum { + VIR_DOMAIN_DIRTYLIMIT_VCPU = 1 << 0,/* render specified virtual CPU for + dirty page rate upper limit (Since: 9.6.0) */ + VIR_DOMAIN_DIRTYLIMIT_ALL = 1 << 1, /* render all virtual CPU for dirty + page rate upper limit (Since: 9.6.0) */ +} virDomainDirtyLimitFlags; + +int virDomainSetVcpuDirtyLimit(virDomainPtr domain, + int vcpu, + unsigned long long rate, + unsigned int flags); #endif /* LIBVIRT_DOMAIN_H */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index 5219344b72..e61b9efca5 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1448,6 +1448,12 @@ typedef int int *fds, unsigned int flags); +typedef int +(*virDrvDomainSetVcpuDirtyLimit)(virDomainPtr domain, + int vcpu, + unsigned long long rate, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; /** @@ -1720,4 +1726,5 @@ struct _virHypervisorDriver { virDrvDomainGetMessages domainGetMessages; virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc; virDrvDomainFDAssociate domainFDAssociate; + virDrvDomainSetVcpuDirtyLimit domainSetVcpuDirtyLimit; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index ec42bb9a53..878d2a6d8c 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -14057,5 +14057,59 @@ virDomainFDAssociate(virDomainPtr domain, error: virDispatchError(conn); +} + +/** + * virDomainSetVcpuDirtyLimit: + * @domain: pointer to domain object, or NULL for Domain0 + * @vcpu: mandatory parameter only if the specified index of the + * virtual CPU is limited; ignored otherwise. + * @rate: upper limit of dirty page rate (MB/s) for virtual CPUs + * @flags: bitwise-OR of supported virDomainDirtyLimitFlags + * + * Dynamically set the upper dirty page rate limit of the virtual CPUs. + * + * Returns 0 in case of success, -1 in case of failure. + * + * Since: 9.6.0 + */ +int +virDomainSetVcpuDirtyLimit(virDomainPtr domain, + int vcpu, + unsigned long long rate, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "vcpu=%d, dirty page rate limit=%lld", + vcpu, rate); + + virCheckFlags(VIR_DOMAIN_DIRTYLIMIT_VCPU | + VIR_DOMAIN_DIRTYLIMIT_ALL, -1); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + virCheckPositiveArgGoto(rate, error); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYLIMIT_VCPU, + VIR_DOMAIN_DIRTYLIMIT_ALL, + error); + + if (conn->driver->domainSetVcpuDirtyLimit) { + int ret; + ret = conn->driver->domainSetVcpuDirtyLimit(domain, vcpu, rate, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); return -1; } diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 80742f268e..6fc01b518f 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -932,4 +932,9 @@ LIBVIRT_9.0.0 { virDomainFDAssociate; } LIBVIRT_8.5.0; +LIBVIRT_9.6.0 { + global: + virDomainSetVcpuDirtyLimit; +} LIBVIRT_9.0.0; + # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 65ec239fb7..4d7682eb32 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8126,6 +8126,7 @@ static virHypervisorDriver hypervisor_driver = { .domainStartDirtyRateCalc = remoteDomainStartDirtyRateCalc, /* 7.2.0 */ .domainSetLaunchSecurityState = remoteDomainSetLaunchSecurityState, /* 8.0.0 */ .domainFDAssociate = remoteDomainFDAssociate, /* 9.0.0 */ + .domainSetVcpuDirtyLimit = remoteDomainSetVcpuDirtyLimit, /* 9.6.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 5d86a51116..f0b7f0a5fa 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3935,6 +3935,14 @@ struct remote_domain_fd_associate_args { remote_nonnull_string name; unsigned int flags; }; + +struct remote_domain_set_vcpu_dirty_limit_args { + remote_nonnull_domain dom; + int vcpu; + unsigned hyper rate; + unsigned int flags; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -6974,5 +6982,10 @@ enum remote_procedure { * @generate: none * @acl: domain:write */ - REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443 + REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, + /** + * @generate: both + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_SET_VCPU_DIRTY_LIMIT = 444 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 3c6c230a16..f7543ec667 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3273,6 +3273,12 @@ struct remote_domain_fd_associate_args { remote_nonnull_string name; u_int flags; }; +struct remote_domain_set_vcpu_dirty_limit_args { + remote_nonnull_domain dom; + int vcpu; + uint64_t rate; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3717,4 +3723,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_RESTORE_PARAMS = 441, REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, + REMOTE_PROC_DOMAIN_SET_VCPU_DIRTY_LIMIT = 444, }; -- 2.38.5

On Tue, Aug 02, 2022 at 22:13:40 +0800, ~hyman wrote:
From: Hyman Huang(黄勇) <yong.huang@smartx.com>
Introduce virDomainSetVcpuDirtyLimit API to set upper limit of dirty page rate.
Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- include/libvirt/libvirt-domain.h | 16 ++++++++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 54 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 ++++++++- src/remote_protocol-structs | 7 +++++ 7 files changed, 104 insertions(+), 1 deletion(-)
[...]
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index ec42bb9a53..878d2a6d8c 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -14057,5 +14057,59 @@ virDomainFDAssociate(virDomainPtr domain,
error: virDispatchError(conn); +} + +/** + * virDomainSetVcpuDirtyLimit: + * @domain: pointer to domain object, or NULL for Domain0
This API certainly will not work with Domain0 as it's not being implemented for XEN, so please remove that part.
+ * @vcpu: mandatory parameter only if the specified index of the + * virtual CPU is limited; ignored otherwise. + * @rate: upper limit of dirty page rate (MB/s) for virtual CPUs + * @flags: bitwise-OR of supported virDomainDirtyLimitFlags + * + * Dynamically set the upper dirty page rate limit of the virtual CPUs.
While you've arbitrarily chose to not implement persisting of this configuration into the XML, it certainly is a possibility that this might be useful. As of such you should add the appropriate flags ( VIR_DOMAIN_AFFECT_CURRENT/VIR_DOMAIN_AFFECT_LIVE/VIR_DOMAIN_AFFECT_CONFIG) (Note that these take up the first two bits of flags). You then add logic that allows the API only when the VM is live and live update is requested later in the code implementing it. That way the API will stay flexible. Additionally the description of what this actually does doesn't feel sufficient. Please enhance the documentation to clearly state what's going on and what impact it has on the VM itself. Also what is the reason for expressing 'rate' as MB/s? Generally bytes/s have definitely enough range in a 64bit variable and you don't get into trouble of having to check it when you need to multiply up to bytes/s. You can add a disclaimer that hypervisors are free to round up/down to the nearest mebibyte/s
+ * + * Returns 0 in case of success, -1 in case of failure. + * + * Since: 9.6.0 + */ +int +virDomainSetVcpuDirtyLimit(virDomainPtr domain, + int vcpu, + unsigned long long rate, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "vcpu=%d, dirty page rate limit=%lld",
'rate' is _unsigned_, but the format substitution calls for 'lld'.
+ vcpu, rate);
'flags' are not logged.
+ + virCheckFlags(VIR_DOMAIN_DIRTYLIMIT_VCPU | + VIR_DOMAIN_DIRTYLIMIT_ALL, -1);
None other API uses virCheckFlags in this file. It's used in the implementation. Additionally this would skip dispatching the error in the first place.
+ + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + virCheckPositiveArgGoto(rate, error);
Since you request 'rate' to be positive, maybe you can use '0' as a special case to cancel the limit instead of adding a new api.
+ + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYLIMIT_VCPU, + VIR_DOMAIN_DIRTYLIMIT_ALL, + error); + + if (conn->driver->domainSetVcpuDirtyLimit) { + int ret; + ret = conn->driver->domainSetVcpuDirtyLimit(domain, vcpu, rate, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); return -1; }

Sorry for the late reply. On Mon, Jul 31, 2023 at 4:11 PM Peter Krempa <pkrempa@redhat.com> wrote:
On Tue, Aug 02, 2022 at 22:13:40 +0800, ~hyman wrote:
From: Hyman Huang(黄勇) <yong.huang@smartx.com>
Introduce virDomainSetVcpuDirtyLimit API to set upper limit of dirty page rate.
Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- include/libvirt/libvirt-domain.h | 16 ++++++++++ src/driver-hypervisor.h | 7 +++++ src/libvirt-domain.c | 54 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 5 +++ src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 15 ++++++++- src/remote_protocol-structs | 7 +++++ 7 files changed, 104 insertions(+), 1 deletion(-)
[...]
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index ec42bb9a53..878d2a6d8c 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -14057,5 +14057,59 @@ virDomainFDAssociate(virDomainPtr domain,
error: virDispatchError(conn); +} + +/** + * virDomainSetVcpuDirtyLimit: + * @domain: pointer to domain object, or NULL for Domain0
This API certainly will not work with Domain0 as it's not being implemented for XEN, so please remove that part.
Ok
+ * @vcpu: mandatory parameter only if the specified index of the + * virtual CPU is limited; ignored otherwise. + * @rate: upper limit of dirty page rate (MB/s) for virtual CPUs + * @flags: bitwise-OR of supported virDomainDirtyLimitFlags + * + * Dynamically set the upper dirty page rate limit of the virtual CPUs.
While you've arbitrarily chose to not implement persisting of this
I'm also tangled in it when writing the first version, so you prefer to implement persisting. I'll introduce the limit info in the struct _virDomainVcpuDef to track the state and enforce the dirty limit setup after launching VM next version, what do you think? Like the 'virsh setvcpu' does.
configuration into the XML, it certainly is a possibility that this might be useful. As of such you should add the appropriate flags ( VIR_DOMAIN_AFFECT_CURRENT/VIR_DOMAIN_AFFECT_LIVE/VIR_DOMAIN_AFFECT_CONFIG) (Note that these take up the first two bits of flags).
Ok, get it, introduce it in the next version.
You then add logic that allows the API only when the VM is live and live update is requested later in the code implementing it.
That way the API will stay flexible.
Additionally the description of what this actually does doesn't feel sufficient. Please enhance the documentation to clearly state what's going on and what impact it has on the VM itself.
OK, I'll enrich the comment.
Also what is the reason for expressing 'rate' as MB/s? Generally bytes/s have definitely enough range in a 64bit variable and you don't get into trouble of having to check it when you need to multiply up to bytes/s.
You can add a disclaimer that hypervisors are free to round up/down to the nearest mebibyte/s
+ * + * Returns 0 in case of success, -1 in case of failure. + * + * Since: 9.6.0 + */ +int +virDomainSetVcpuDirtyLimit(virDomainPtr domain, + int vcpu, + unsigned long long rate, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "vcpu=%d, dirty page rate limit=%lld",
'rate' is _unsigned_, but the format substitution calls for 'lld'.
+ vcpu, rate);
'flags' are not logged.
Get it.
+ + virCheckFlags(VIR_DOMAIN_DIRTYLIMIT_VCPU | + VIR_DOMAIN_DIRTYLIMIT_ALL, -1);
None other API uses virCheckFlags in this file. It's used in the implementation. Additionally this would skip dispatching the error in the first place.
+ + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + virCheckPositiveArgGoto(rate, error);
Since you request 'rate' to be positive, maybe you can use '0' as a special case to cancel the limit instead of adding a new api.
Yes, this simplifies the implementation absolutely, I'll drop the cancellation API and reuse this in the next version.
+ + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYLIMIT_VCPU, + VIR_DOMAIN_DIRTYLIMIT_ALL, + error); + + if (conn->driver->domainSetVcpuDirtyLimit) { + int ret; + ret = conn->driver->domainSetVcpuDirtyLimit(domain, vcpu, rate, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); return -1; }
-- Best regards

From: Hyman Huang(黄勇) <yong.huang@smartx.com> Implement qemuDomainSetVcpuDirtyLimit which set dirty page rate upper limit. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 13 +++++++++ src/qemu/qemu_monitor.h | 5 ++++ src/qemu/qemu_monitor_json.c | 45 +++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 ++++ 5 files changed, 120 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 497923ffee..61b992fc51 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19912,6 +19912,57 @@ qemuDomainFDAssociate(virDomainPtr domain, return ret; } +static int +qemuDomainSetVcpuDirtyLimit(virDomainPtr domain, + int vcpu, + unsigned long long rate, + unsigned int flags) +{ + virDomainObj *vm = NULL; + qemuDomainObjPrivate *priv; + int ret = -1; + + if (!(vm = qemuDomainObjFromDomain(domain))) + return -1; + + if (virDomainSetVcpuDirtyLimitEnsureACL(domain->conn, vm->def)) + goto cleanup; + + priv = vm->privateData; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VCPU_DIRTY_LIMIT)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("QEMU does not support setting dirty page rate limit")); + goto cleanup; + } + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + qemuDomainObjEnterMonitor(vm); + if (flags & VIR_DOMAIN_DIRTYLIMIT_VCPU) { + if (vcpu < 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Require cpu index to limit dirty page rate")); + goto endjob; + } + ret = qemuMonitorSetVcpuDirtyLimit(priv->mon, vcpu, rate); + VIR_DEBUG("Set vcpu[%d] dirty page rate limit %lld", vcpu, rate); + } else { + ret = qemuMonitorSetVcpuDirtyLimit(priv->mon, -1, rate); + VIR_DEBUG("Set all vcpus dirty page rate limit %lld of vm", rate); + } + qemuDomainObjExitMonitor(vm); + + endjob: + virDomainObjEndJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, @@ -20162,6 +20213,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */ .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */ .domainFDAssociate = qemuDomainFDAssociate, /* 9.0.0 */ + .domainSetVcpuDirtyLimit = qemuDomainSetVcpuDirtyLimit, /* 9.6.0 */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c680c4b804..87926edec6 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4505,3 +4505,16 @@ qemuMonitorGetStatsByQOMPath(virJSONValue *arr, return NULL; } + + +int +qemuMonitorSetVcpuDirtyLimit(qemuMonitor *mon, + int vcpu, + unsigned long long rate) +{ + VIR_DEBUG("set vcpu %d dirty page rate limit %lld", vcpu, rate); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSetVcpuDirtyLimit(mon, vcpu, rate); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6c590933aa..07a05365cf 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1579,3 +1579,8 @@ qemuMonitorExtractQueryStats(virJSONValue *info); virJSONValue * qemuMonitorGetStatsByQOMPath(virJSONValue *arr, char *qom_path); + +int +qemuMonitorSetVcpuDirtyLimit(qemuMonitor *mon, + int vcpu, + unsigned long long rate); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d9e9a4481c..732366ab29 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8889,3 +8889,48 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon, return virJSONValueObjectStealArray(reply, "return"); } + +/** + * qemuMonitorJSONSetVcpuDirtyLimit: + * @mon: monitor object + * @vcpu: virtual cpu index to be set, -1 means all virtual cpus + * @rate: dirty page rate upper limit to be set + * + * Returns -1 on failure. + */ +int +qemuMonitorJSONSetVcpuDirtyLimit(qemuMonitor *mon, + int vcpu, + unsigned long long rate) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + if (vcpu < -1) + return -1; + + if (vcpu >= 0) { + /* set vcpu dirty page rate limit */ + if (!(cmd = qemuMonitorJSONMakeCommand("set-vcpu-dirty-limit", + "i:cpu-index", vcpu, + "U:dirty-rate", rate, + NULL))) { + return -1; + } + } else if (vcpu == -1) { + /* set vm dirty page rate limit */ + if (!(cmd = qemuMonitorJSONMakeCommand("set-vcpu-dirty-limit", + "U:dirty-rate", rate, + 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 06023b98ea..89f61b3052 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -825,3 +825,8 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon, qemuMonitorQueryStatsTargetType target, char **vcpus, GPtrArray *providers); + +int +qemuMonitorJSONSetVcpuDirtyLimit(qemuMonitor *mon, + int vcpu, + unsigned long long rate); -- 2.38.5

On Wed, Aug 03, 2022 at 00:27:47 +0800, ~hyman wrote:
From: Hyman Huang(黄勇) <yong.huang@smartx.com>
Implement qemuDomainSetVcpuDirtyLimit which set dirty page rate upper limit.
Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- src/qemu/qemu_driver.c | 52 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 13 +++++++++ src/qemu/qemu_monitor.h | 5 ++++ src/qemu/qemu_monitor_json.c | 45 +++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 5 ++++ 5 files changed, 120 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 497923ffee..61b992fc51 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19912,6 +19912,57 @@ qemuDomainFDAssociate(virDomainPtr domain, return ret; }
+static int +qemuDomainSetVcpuDirtyLimit(virDomainPtr domain, + int vcpu, + unsigned long long rate, + unsigned int flags) +{ + virDomainObj *vm = NULL; + qemuDomainObjPrivate *priv; + int ret = -1; + + if (!(vm = qemuDomainObjFromDomain(domain))) + return -1; + + if (virDomainSetVcpuDirtyLimitEnsureACL(domain->conn, vm->def)) + goto cleanup; + + priv = vm->privateData; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VCPU_DIRTY_LIMIT)) {
note that priv->qemuCaps is NULL when the VM is not running, so you'll get ...
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("QEMU does not support setting dirty page rate limit"));
... this error.
+ goto cleanup; + } + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0)
Instead of this one. You need to reorder these, but that'll be needed regardless when you add the logic for handlign the virDomainModificationImpact flags.
+ goto endjob; + + qemuDomainObjEnterMonitor(vm); + if (flags & VIR_DOMAIN_DIRTYLIMIT_VCPU) { + if (vcpu < 0) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("Require cpu index to limit dirty page rate"));
So is there a need to pass 'vcpu' as 'int' rather than unsigned int?
+ goto endjob;
This skips over the call to qemuDomainObjExitMonitor;
+ } + ret = qemuMonitorSetVcpuDirtyLimit(priv->mon, vcpu, rate); + VIR_DEBUG("Set vcpu[%d] dirty page rate limit %lld", vcpu, rate); + } else { + ret = qemuMonitorSetVcpuDirtyLimit(priv->mon, -1, rate); + VIR_DEBUG("Set all vcpus dirty page rate limit %lld of vm", rate); + } + qemuDomainObjExitMonitor(vm); + + endjob: + virDomainObjEndJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +}
static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, @@ -20162,6 +20213,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainStartDirtyRateCalc = qemuDomainStartDirtyRateCalc, /* 7.2.0 */ .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */ .domainFDAssociate = qemuDomainFDAssociate, /* 9.0.0 */ + .domainSetVcpuDirtyLimit = qemuDomainSetVcpuDirtyLimit, /* 9.6.0 */ };
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index c680c4b804..87926edec6 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4505,3 +4505,16 @@ qemuMonitorGetStatsByQOMPath(virJSONValue *arr,
return NULL; } + + +int +qemuMonitorSetVcpuDirtyLimit(qemuMonitor *mon, + int vcpu, + unsigned long long rate) +{ + VIR_DEBUG("set vcpu %d dirty page rate limit %lld", vcpu, rate);
'rate' is unsigned.
+ + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONSetVcpuDirtyLimit(mon, vcpu, rate); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 6c590933aa..07a05365cf 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1579,3 +1579,8 @@ qemuMonitorExtractQueryStats(virJSONValue *info); virJSONValue * qemuMonitorGetStatsByQOMPath(virJSONValue *arr, char *qom_path); + +int +qemuMonitorSetVcpuDirtyLimit(qemuMonitor *mon, + int vcpu, + unsigned long long rate); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d9e9a4481c..732366ab29 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8889,3 +8889,48 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon,
return virJSONValueObjectStealArray(reply, "return"); } + +/** + * qemuMonitorJSONSetVcpuDirtyLimit: + * @mon: monitor object + * @vcpu: virtual cpu index to be set, -1 means all virtual cpus + * @rate: dirty page rate upper limit to be set + * + * Returns -1 on failure. + */ +int +qemuMonitorJSONSetVcpuDirtyLimit(qemuMonitor *mon, + int vcpu, + unsigned long long rate) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + if (vcpu < -1) + return -1;
This doesn't report an error, why everything else does. Also this doesn't really seem to be needed.
+ + if (vcpu >= 0) { + /* set vcpu dirty page rate limit */ + if (!(cmd = qemuMonitorJSONMakeCommand("set-vcpu-dirty-limit", + "i:cpu-index", vcpu, + "U:dirty-rate", rate, + NULL))) { + return -1; + } + } else if (vcpu == -1) { + /* set vm dirty page rate limit */ + if (!(cmd = qemuMonitorJSONMakeCommand("set-vcpu-dirty-limit", + "U:dirty-rate", rate, + NULL))) {
Since both use the same command, you can use e.g. 'k:cpu-index': * k: signed integer value, omitted if negative And get rid of the whole if/elseif thing.
+ 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 06023b98ea..89f61b3052 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -825,3 +825,8 @@ qemuMonitorJSONQueryStats(qemuMonitor *mon, qemuMonitorQueryStatsTargetType target, char **vcpus, GPtrArray *providers); + +int +qemuMonitorJSONSetVcpuDirtyLimit(qemuMonitor *mon, + int vcpu, + unsigned long long rate); -- 2.38.5

From: Hyman Huang(黄勇) <yong.huang@smartx.com> set-vcpu-dirty-limit/cancel-vcpu-dirty-limit/query-vcpu-dirty-limit were introduced since qemu >=7.1.0. Introduce corresponding capability. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml | 1 + tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml | 1 + tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml | 1 + tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml | 1 + tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml | 1 + 11 files changed, 12 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index f80bdb579d..6e0c095b55 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -697,6 +697,7 @@ VIR_ENUM_IMPL(virQEMUCaps, /* 450 */ "run-with.async-teardown", /* QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN */ + "set-vcpu-dirty-limit", /* QEMU_CAPS_VCPU_DIRTY_LIMIT */ ); @@ -1221,6 +1222,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = { { "calc-dirty-rate", QEMU_CAPS_CALC_DIRTY_RATE }, { "query-stats", QEMU_CAPS_QUERY_STATS }, { "query-stats-schemas", QEMU_CAPS_QUERY_STATS_SCHEMAS }, + { "set-vcpu-dirty-limit", QEMU_CAPS_VCPU_DIRTY_LIMIT }, }; struct virQEMUCapsStringFlags virQEMUCapsMigration[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index c72f73a161..9e96f548af 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -676,6 +676,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ /* 450 */ QEMU_CAPS_RUN_WITH_ASYNC_TEARDOWN, /* asynchronous teardown -run-with async-teardown=on|off */ + QEMU_CAPS_VCPU_DIRTY_LIMIT, /* 'set-vcpu-dirty-limit' QMP command present */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml b/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml index 3ff7a88cd2..f333df6599 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0_ppc64.xml @@ -158,6 +158,7 @@ <flag name='virtio-crypto'/> <flag name='pvpanic-pci'/> <flag name='virtio-gpu.blob'/> + <flag name='set-vcpu-dirty-limit'/> <version>7001000</version> <microcodeVersion>42900244</microcodeVersion> <package>v7.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml index 4e2addd76b..20e10b3090 100644 --- a/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.1.0_x86_64.xml @@ -195,6 +195,7 @@ <flag name='virtio-crypto'/> <flag name='pvpanic-pci'/> <flag name='virtio-gpu.blob'/> + <flag name='set-vcpu-dirty-limit'/> <version>7001000</version> <microcodeVersion>43100244</microcodeVersion> <package>v7.1.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml b/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml index 06f8c5801f..50e1d6c359 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_ppc.xml @@ -153,6 +153,7 @@ <flag name='virtio-crypto'/> <flag name='pvpanic-pci'/> <flag name='virtio-gpu.blob'/> + <flag name='set-vcpu-dirty-limit'/> <version>7002000</version> <microcodeVersion>0</microcodeVersion> <package>qemu-7.2.0-6.fc37</package> diff --git a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml index 0007a33dca..d804bb51e1 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64+hvf.xml @@ -199,6 +199,7 @@ <flag name='cryptodev-backend-lkcf'/> <flag name='pvpanic-pci'/> <flag name='virtio-gpu.blob'/> + <flag name='set-vcpu-dirty-limit'/> <version>7002000</version> <microcodeVersion>43100245</microcodeVersion> <package>v7.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml index e298cbd9b1..618e2e7778 100644 --- a/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_7.2.0_x86_64.xml @@ -199,6 +199,7 @@ <flag name='cryptodev-backend-lkcf'/> <flag name='pvpanic-pci'/> <flag name='virtio-gpu.blob'/> + <flag name='set-vcpu-dirty-limit'/> <version>7002000</version> <microcodeVersion>43100245</microcodeVersion> <package>v7.2.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml b/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml index 987962ca41..0643fd8054 100644 --- a/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml +++ b/tests/qemucapabilitiesdata/caps_8.0.0_riscv64.xml @@ -140,6 +140,7 @@ <flag name='virtio-crypto'/> <flag name='pvpanic-pci'/> <flag name='virtio-gpu.blob'/> + <flag name='set-vcpu-dirty-limit'/> <version>7002050</version> <microcodeVersion>0</microcodeVersion> <package>v7.2.0-333-g222059a0fc</package> diff --git a/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml index c43c209328..1e0bc96f88 100644 --- a/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.0.0_x86_64.xml @@ -203,6 +203,7 @@ <flag name='virtio-gpu.blob'/> <flag name='rbd-encryption-layering'/> <flag name='rbd-encryption-luks-any'/> + <flag name='set-vcpu-dirty-limit'/> <version>8000000</version> <microcodeVersion>43100244</microcodeVersion> <package>v8.0.0</package> diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml index 35751ed441..6d5e6ee76f 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_s390x.xml @@ -114,6 +114,7 @@ <flag name='rbd-encryption-layering'/> <flag name='rbd-encryption-luks-any'/> <flag name='run-with.async-teardown'/> + <flag name='set-vcpu-dirty-limit'/> <version>8000050</version> <microcodeVersion>39100245</microcodeVersion> <package>v8.0.0-1270-g1c12355b</package> diff --git a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml index e656a2024a..ca8b5d056c 100644 --- a/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_8.1.0_x86_64.xml @@ -204,6 +204,7 @@ <flag name='rbd-encryption-luks-any'/> <flag name='qcow2-discard-no-unref'/> <flag name='run-with.async-teardown'/> + <flag name='set-vcpu-dirty-limit'/> <version>8000050</version> <microcodeVersion>43100245</microcodeVersion> <package>v8.0.0-2835-g361d539735</package> -- 2.38.5

From: Hyman Huang(黄勇) <yong.huang@smartx.com> Extend vcpuinfo api to display dirty page rate upper limit info, which is set by 'limit-dirty-page-rate'. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- src/remote/remote_daemon_dispatch.c | 2 ++ src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 2 ++ tools/virsh-domain.c | 5 +++++ 4 files changed, 11 insertions(+) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 7144e9e7ca..edf6386653 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -2893,6 +2893,8 @@ remoteDispatchDomainGetVcpus(virNetServer *server G_GNUC_UNUSED, ret->info.info_val[i].state = info[i].state; ret->info.info_val[i].cpu_time = info[i].cpuTime; ret->info.info_val[i].cpu = info[i].cpu; + ret->info.info_val[i].limit = info[i].limit; + ret->info.info_val[i].current = info[i].current; } /* Don't need to allocate/copy the cpumaps if we make the reasonable diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 4d7682eb32..c03e9e862b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -2147,6 +2147,8 @@ remoteDomainGetVcpus(virDomainPtr domain, info[i].state = ret.info.info_val[i].state; info[i].cpuTime = ret.info.info_val[i].cpu_time; info[i].cpu = ret.info.info_val[i].cpu; + info[i].limit = ret.info.info_val[i].limit; + info[i].current = ret.info.info_val[i].current; } for (i = 0; i < ret.cpumaps.cpumaps_len; ++i) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f0b7f0a5fa..633b9af74e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -416,6 +416,8 @@ struct remote_vcpu_info { int state; unsigned hyper cpu_time; int cpu; + unsigned hyper limit; + unsigned hyper current; }; /* Wire encoding of virTypedParameter. diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index ba2309eb3c..b51bd7a3d0 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6935,6 +6935,11 @@ cmdVcpuinfo(vshControl *ctl, const vshCmd *cmd) maxcpu, pretty) < 0) return false; + if (cpuinfo[n].limit != 0) { + vshPrint(ctl, "%-15s %lld\n", _("DirtyRate limit:"), cpuinfo[n].limit); + vshPrint(ctl, "%-15s %lld\n", _("DirtyRate current:"), cpuinfo[n].current); + } + if (n < (ncpus - 1)) vshPrint(ctl, "\n"); } -- 2.38.5

On Sat, Aug 13, 2022 at 11:11:15 +0800, ~hyman wrote:
From: Hyman Huang(黄勇) <yong.huang@smartx.com>
Extend vcpuinfo api to display dirty page rate upper limit info, which is set by 'limit-dirty-page-rate'.
Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- src/remote/remote_daemon_dispatch.c | 2 ++ src/remote/remote_driver.c | 2 ++ src/remote/remote_protocol.x | 2 ++ tools/virsh-domain.c | 5 +++++ 4 files changed, 11 insertions(+) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index f0b7f0a5fa..633b9af74e 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -416,6 +416,8 @@ struct remote_vcpu_info { int state; unsigned hyper cpu_time; int cpu; + unsigned hyper limit; + unsigned hyper current; };
NACK, see reasoning in 6/11

From: Hyman Huang(黄勇) <yong.huang@smartx.com> Introduce limit-dirty-page-rate virsh api to set dirty page rate upper limit for virtual CPUs: The following is the usage: $ virsh limit-dirty-page-rate <domain> <rate> [--vcpu <number>] Set the specified index of vcpu if 'vcpu' option is specified, set all virtual CPUs of a VM otherwise. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- tools/virsh-domain.c | 92 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fb54562eb6..ba2309eb3c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13817,6 +13817,92 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd) return true; } +#define IGNORED_CPU_INDEX -1 + +/* + * "limit-dirty-page-rate" command + */ +static const vshCmdInfo info_limit_dirty_page_rate[] = { + {.name = "help", + .data = N_("Set dirty page rate upper limit") + }, + {.name = "desc", + .data = N_("Set dirty page rate upper limit, " + "require dirty-ring size configured") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_limit_dirty_page_rate[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "rate", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("Upper limit of dirty page rate (MB/s) for " + "virtual CPUs") + }, + {.name = "vcpu", + .type = VSH_OT_INT, + .help = N_("Index of a virtual CPU") + }, + {.name = NULL} +}; + +static bool +cmdLimitDirtyPageRate(vshControl *ctl, const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + int vcpu_idx = -1; + unsigned long long rate = 0; + bool vcpu = vshCommandOptBool(cmd, "vcpu"); + unsigned int flags = 0; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vcpu) { + if (vshCommandOptInt(ctl, cmd, "vcpu", &vcpu_idx) < 0) + return false; + + if (vcpu_idx < 0) { + vshError(ctl, "%s", _("Invalid vcpu index, using --vcpu " + "to specify cpu index")); + return false; + } + } + + if (vshCommandOptULongLong(ctl, cmd, "rate", &rate) < 0) + return false; + + if (!rate) { + vshError(ctl, "%s", _("Invalid dirty page rate limit")); + return false; + } + + if (vcpu) { + /* set specified vcpu dirty page rate limit of vm */ + if (virDomainSetVcpuDirtyLimit(dom, vcpu_idx, + rate, flags | VIR_DOMAIN_DIRTYLIMIT_VCPU) < 0) + return false; + g_autofree char *info = + g_strdup_printf("Set vcpu[%d] dirty page rate upper " + "limit %lld(MB/s) successfully", + vcpu_idx, rate); + vshPrintExtra(ctl, _("%1$s\n"), info); + } else { + /* set all vcpu dirty page rate limit of vm */ + if (virDomainSetVcpuDirtyLimit(dom, IGNORED_CPU_INDEX, + rate, flags | VIR_DOMAIN_DIRTYLIMIT_ALL) < 0) + return false; + g_autofree char *info = + g_strdup_printf("Set dirty page rate limit %lld(MB/s) " + "on all virtual CPUs successfully", + rate); + vshPrintExtra(ctl, _("%1$s\n"), info); + } + + return true; +} const vshCmdDef domManagementCmds[] = { {.name = "attach-device", @@ -14481,5 +14567,11 @@ const vshCmdDef domManagementCmds[] = { .info = info_dom_fd_associate, .flags = 0 }, + {.name = "limit-dirty-page-rate", + .handler = cmdLimitDirtyPageRate, + .opts = opts_limit_dirty_page_rate, + .info = info_limit_dirty_page_rate, + .flags = 0 + }, {.name = NULL} }; -- 2.38.5

On Mon, Aug 08, 2022 at 23:41:27 +0800, ~hyman wrote:
From: Hyman Huang(黄勇) <yong.huang@smartx.com>
Introduce limit-dirty-page-rate virsh api to set dirty page rate upper limit for virtual CPUs:
The following is the usage: $ virsh limit-dirty-page-rate <domain> <rate> [--vcpu <number>]
Set the specified index of vcpu if 'vcpu' option is specified, set all virtual CPUs of a VM otherwise.
Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- tools/virsh-domain.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
missing addition to docs/manpages/virsh.rst
1 file changed, 92 insertions(+)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index fb54562eb6..ba2309eb3c 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13817,6 +13817,92 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd) return true; }
+#define IGNORED_CPU_INDEX -1
Too generic name. Also no need for the definition most likely.
+ +/* + * "limit-dirty-page-rate" command + */ +static const vshCmdInfo info_limit_dirty_page_rate[] = { + {.name = "help", + .data = N_("Set dirty page rate upper limit") + }, + {.name = "desc", + .data = N_("Set dirty page rate upper limit, " + "require dirty-ring size configured")
This limitation is not documented anywhere else.
+ }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_limit_dirty_page_rate[] = { + VIRSH_COMMON_OPT_DOMAIN_FULL(0), + {.name = "rate", + .type = VSH_OT_INT, + .flags = VSH_OFLAG_REQ, + .help = N_("Upper limit of dirty page rate (MB/s) for " + "virtual CPUs") + }, + {.name = "vcpu", + .type = VSH_OT_INT, + .help = N_("Index of a virtual CPU") + }, + {.name = NULL} +}; + +static bool +cmdLimitDirtyPageRate(vshControl *ctl, const vshCmd *cmd) +{ + g_autoptr(virshDomain) dom = NULL; + int vcpu_idx = -1; + unsigned long long rate = 0; + bool vcpu = vshCommandOptBool(cmd, "vcpu"); + unsigned int flags = 0; + + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) + return false; + + if (vcpu) { + if (vshCommandOptInt(ctl, cmd, "vcpu", &vcpu_idx) < 0) + return false; + + if (vcpu_idx < 0) { + vshError(ctl, "%s", _("Invalid vcpu index, using --vcpu " + "to specify cpu index")); + return false; + } + } + + if (vshCommandOptULongLong(ctl, cmd, "rate", &rate) < 0) + return false; + + if (!rate) {
Checks for 0 with integers that can have arbitrary numeric value should be explicit "if (rate == 0)"...
+ vshError(ctl, "%s", _("Invalid dirty page rate limit")); + return false; + } + + if (vcpu) { + /* set specified vcpu dirty page rate limit of vm */ + if (virDomainSetVcpuDirtyLimit(dom, vcpu_idx, + rate, flags | VIR_DOMAIN_DIRTYLIMIT_VCPU) < 0) + return false; + g_autofree char *info = + g_strdup_printf("Set vcpu[%d] dirty page rate upper " + "limit %lld(MB/s) successfully", + vcpu_idx, rate);
This is not translatable. Make sure to always construct the message within vshPRintExtra's second argument which is wrapped with the gettext macro.
+ vshPrintExtra(ctl, _("%1$s\n"), info); + } else { + /* set all vcpu dirty page rate limit of vm */ + if (virDomainSetVcpuDirtyLimit(dom, IGNORED_CPU_INDEX, + rate, flags | VIR_DOMAIN_DIRTYLIMIT_ALL) < 0) + return false; + g_autofree char *info = + g_strdup_printf("Set dirty page rate limit %lld(MB/s) " + "on all virtual CPUs successfully", + rate);
This is not translatable.
+ vshPrintExtra(ctl, _("%1$s\n"), info); + } + + return true; +}
const vshCmdDef domManagementCmds[] = { {.name = "attach-device", @@ -14481,5 +14567,11 @@ const vshCmdDef domManagementCmds[] = { .info = info_dom_fd_associate, .flags = 0 }, + {.name = "limit-dirty-page-rate", + .handler = cmdLimitDirtyPageRate, + .opts = opts_limit_dirty_page_rate, + .info = info_limit_dirty_page_rate, + .flags = 0 + }, {.name = NULL} }; -- 2.38.5

From: Hyman Huang(黄勇) <yong.huang@smartx.com> Add cancel option of limit-dirty-page-rate api to cancel dirty page rate upper limit for virtual CPUs. The following is the usage: $ virsh limit-dirty-page-rate <domain> [--rate <number>] \ [--vcpu <number>] [--cancel] Specify the 'cancel' option to cancel dirty page rate upper limit. Cancel the specified index of vcpu if 'vcpu' option is specified, cancel all virtual CPUs of a VM otherwise. Cancelling dirty page rate upper limit does not need 'rate' option. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- tools/virsh-domain.c | 82 +++++++++++++++++++++++++++++--------------- 1 file changed, 54 insertions(+), 28 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index b51bd7a3d0..7f112efdfe 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -13829,10 +13829,10 @@ cmdDomDirtyRateCalc(vshControl *ctl, const vshCmd *cmd) */ static const vshCmdInfo info_limit_dirty_page_rate[] = { {.name = "help", - .data = N_("Set dirty page rate upper limit") + .data = N_("Set or cancel dirty page rate upper limit") }, {.name = "desc", - .data = N_("Set dirty page rate upper limit, " + .data = N_("Set or cancel dirty page rate upper limit, " "require dirty-ring size configured") }, {.name = NULL} @@ -13842,7 +13842,6 @@ static const vshCmdOptDef opts_limit_dirty_page_rate[] = { VIRSH_COMMON_OPT_DOMAIN_FULL(0), {.name = "rate", .type = VSH_OT_INT, - .flags = VSH_OFLAG_REQ, .help = N_("Upper limit of dirty page rate (MB/s) for " "virtual CPUs") }, @@ -13850,6 +13849,10 @@ static const vshCmdOptDef opts_limit_dirty_page_rate[] = { .type = VSH_OT_INT, .help = N_("Index of a virtual CPU") }, + {.name = "cancel", + .type = VSH_OT_BOOL, + .help = N_("Cancel dirty page rate upper limit") + }, {.name = NULL} }; @@ -13860,8 +13863,11 @@ cmdLimitDirtyPageRate(vshControl *ctl, const vshCmd *cmd) int vcpu_idx = -1; unsigned long long rate = 0; bool vcpu = vshCommandOptBool(cmd, "vcpu"); + bool cancel = vshCommandOptBool(cmd, "cancel"); unsigned int flags = 0; + VSH_EXCLUSIVE_OPTIONS("cancel", "rate"); + if (!(dom = virshCommandOptDomain(ctl, cmd, NULL))) return false; @@ -13876,34 +13882,54 @@ cmdLimitDirtyPageRate(vshControl *ctl, const vshCmd *cmd) } } - if (vshCommandOptULongLong(ctl, cmd, "rate", &rate) < 0) - return false; - - if (!rate) { - vshError(ctl, "%s", _("Invalid dirty page rate limit")); - return false; - } + if (!cancel) { + if (vshCommandOptULongLong(ctl, cmd, "rate", &rate) < 0) + return false; - if (vcpu) { - /* set specified vcpu dirty page rate limit of vm */ - if (virDomainSetVcpuDirtyLimit(dom, vcpu_idx, - rate, flags | VIR_DOMAIN_DIRTYLIMIT_VCPU) < 0) + if (!rate) { + vshError(ctl, "%s", _("Invalid dirty page rate limit")); return false; - g_autofree char *info = - g_strdup_printf("Set vcpu[%d] dirty page rate upper " - "limit %lld(MB/s) successfully", - vcpu_idx, rate); - vshPrintExtra(ctl, _("%1$s\n"), info); + } + + if (vcpu) { + /* set specified vcpu dirty page rate limit of vm */ + if (virDomainSetVcpuDirtyLimit(dom, vcpu_idx, + rate, flags | VIR_DOMAIN_DIRTYLIMIT_VCPU) < 0) + return false; + g_autofree char *info = + g_strdup_printf("Set vcpu[%d] dirty page rate upper " + "limit %lld(MB/s) successfully", + vcpu_idx, rate); + vshPrintExtra(ctl, _("%1$s\n"), info); + } else { + /* set all vcpu dirty page rate limit of vm */ + if (virDomainSetVcpuDirtyLimit(dom, IGNORED_CPU_INDEX, + rate, flags | VIR_DOMAIN_DIRTYLIMIT_ALL) < 0) + return false; + g_autofree char *info = + g_strdup_printf("Set dirty page rate limit %lld(MB/s) " + "on all virtual CPUs successfully", + rate); + vshPrintExtra(ctl, _("%1$s\n"), info); + } } else { - /* set all vcpu dirty page rate limit of vm */ - if (virDomainSetVcpuDirtyLimit(dom, IGNORED_CPU_INDEX, - rate, flags | VIR_DOMAIN_DIRTYLIMIT_ALL) < 0) - return false; - g_autofree char *info = - g_strdup_printf("Set dirty page rate limit %lld(MB/s) " - "on all virtual CPUs successfully", - rate); - vshPrintExtra(ctl, _("%1$s\n"), info); + if (vcpu) { + /* cancel specified vcpu dirty page rate limit of vm */ + if (virDomainCancelVcpuDirtyLimit(dom, vcpu_idx, + flags | VIR_DOMAIN_DIRTYLIMIT_VCPU) < 0) + return false; + g_autofree char *info = + g_strdup_printf("Cancel vcpu[%d] dirty page rate limit " + "successfully", vcpu); + vshPrintExtra(ctl, _("%1$s\n"), info); + } else { + /* cancel all vcpu dirty page rate limit of vm */ + if (virDomainCancelVcpuDirtyLimit(dom, IGNORED_CPU_INDEX, + flags | VIR_DOMAIN_DIRTYLIMIT_ALL) < 0) + return false; + vshPrintExtra(ctl, "%s", _("Cancel dirty page rate upper " + "limit on all virtual CPUs successfully\n")); + } } return true; -- 2.38.5

From: Hyman Huang(黄勇) <yong.huang@smartx.com> Implement qemuDomainCancelVcpuDirtyLimit to cancel vcpu dirty page rate upper limit. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- src/qemu/qemu_driver.c | 47 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor.c | 11 +++++++++ src/qemu/qemu_monitor.h | 4 +++ src/qemu/qemu_monitor_json.c | 41 +++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 5 files changed, 107 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f7ff3b7098..c4ffacd88c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20019,6 +20019,52 @@ qemuDomainSetVcpuDirtyLimit(virDomainPtr domain, return ret; } +static int +qemuDomainCancelVcpuDirtyLimit(virDomainPtr domain, + int vcpu, + unsigned int flags) +{ + virDomainObj *vm = NULL; + qemuDomainObjPrivate *priv; + int ret = -1; + + if (!(vm = qemuDomainObjFromDomain(domain))) + return -1; + + if (virDomainCancelVcpuDirtyLimitEnsureACL(domain->conn, vm->def)) + goto cleanup; + + priv = vm->privateData; + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VCPU_DIRTY_LIMIT)) { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("QEMU does not support setting vcpu dirty page rate limit")); + goto cleanup; + } + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto cleanup; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + qemuDomainObjEnterMonitor(vm); + if (flags & VIR_DOMAIN_DIRTYLIMIT_VCPU) { + VIR_DEBUG("Cancel vcpu[%d] dirty page rate limit", vcpu); + ret = qemuMonitorCancelVcpuDirtyLimit(priv->mon, vcpu); + } else { + VIR_DEBUG("Cancel all vcpus dirty page rate limit of vm"); + ret = qemuMonitorCancelVcpuDirtyLimit(priv->mon, -1); + } + qemuDomainObjExitMonitor(vm); + + endjob: + virDomainObjEndJob(vm); + + cleanup: + virDomainObjEndAPI(&vm); + return ret; +} + static virHypervisorDriver qemuHypervisorDriver = { .name = QEMU_DRIVER_NAME, .connectURIProbe = qemuConnectURIProbe, @@ -20269,6 +20315,7 @@ static virHypervisorDriver qemuHypervisorDriver = { .domainSetLaunchSecurityState = qemuDomainSetLaunchSecurityState, /* 8.0.0 */ .domainFDAssociate = qemuDomainFDAssociate, /* 9.0.0 */ .domainSetVcpuDirtyLimit = qemuDomainSetVcpuDirtyLimit, /* 9.6.0 */ + .domainCancelVcpuDirtyLimit = qemuDomainCancelVcpuDirtyLimit, /* 9.6.0 */ }; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index a8a6a6e4e2..13cb4f1e79 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4530,3 +4530,14 @@ qemuMonitorQueryVcpuDirtyLimit(qemuMonitor *mon, return qemuMonitorJSONQueryVcpuDirtyLimit(mon, info); } + +int +qemuMonitorCancelVcpuDirtyLimit(qemuMonitor *mon, + int vcpu) +{ + VIR_DEBUG("cancel vcpu %d dirty page rate limit", vcpu); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONCancelVcpuDirtyLimit(mon, vcpu); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 1828bb202a..cbfd4fdaaf 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1601,3 +1601,7 @@ struct _qemuMonitorVcpuDirtyLimitInfo { int qemuMonitorQueryVcpuDirtyLimit(qemuMonitor *mon, qemuMonitorVcpuDirtyLimitInfo *info); + +int +qemuMonitorCancelVcpuDirtyLimit(qemuMonitor *mon, + int vcpu); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index f8406c1857..c0a79dfa97 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8998,3 +8998,44 @@ qemuMonitorJSONQueryVcpuDirtyLimit(qemuMonitor *mon, return qemuMonitorJSONExtractVcpuDirtyLimitInfo(data, info); } + +/** + * qemuMonitorJSONCancelVcpuDirtyLimit: + * @mon: monitor object + * @vcpu: virtual cpu index to be cancelled, -1 means all virtual cpus + * + * Returns -1 on failure. + */ +int +qemuMonitorJSONCancelVcpuDirtyLimit(qemuMonitor *mon, + int vcpu) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + + if (vcpu < -1) + return -1; + + if (vcpu >= 0) { + /* cancel vcpu dirty page rate limit */ + if (!(cmd = qemuMonitorJSONMakeCommand("cancel-vcpu-dirty-limit", + "i:cpu-index", vcpu, + NULL))) { + return -1; + } + } else if (vcpu == -1) { + /* cancel vm dirty page rate limit */ + if (!(cmd = qemuMonitorJSONMakeCommand("cancel-vcpu-dirty-limit", + 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 bd8131508b..0549627c81 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -834,3 +834,7 @@ qemuMonitorJSONSetVcpuDirtyLimit(qemuMonitor *mon, int qemuMonitorJSONQueryVcpuDirtyLimit(qemuMonitor *mon, qemuMonitorVcpuDirtyLimitInfo *info); + +int +qemuMonitorJSONCancelVcpuDirtyLimit(qemuMonitor *mon, + int vcpu); -- 2.38.5

From: Hyman Huang(黄勇) <yong.huang@smartx.com> Extend qemuDomainGetVcpus for getting dirty page rate upper limit info so 'virsh vcpuinfo' api can display it. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- include/libvirt/libvirt-domain.h | 2 ++ src/qemu/qemu_driver.c | 55 ++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index df7deffaa9..4c63d0be7c 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2395,6 +2395,8 @@ struct _virVcpuInfo { int state; /* value from virVcpuState */ unsigned long long cpuTime; /* CPU time used, in nanoseconds */ int cpu; /* real CPU number, or one of the values from virVcpuHostCpuState */ + unsigned long long limit; /* virtual cpu dirty page rate upper limit in MB/s */ + unsigned long long current; /* current virtual cpu dirty page rate in MB/s */ }; /** diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 61b992fc51..f7ff3b7098 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -4599,6 +4599,57 @@ qemuDomainGetEmulatorPinInfo(virDomainPtr dom, return ret; } +static int +qemuDomainGetVcpuDirtyLimit(virDomainObj *vm, + virVcpuInfoPtr info, + int maxinfo) +{ + qemuDomainObjPrivate *priv = vm->privateData; + qemuMonitorVcpuDirtyLimitInfo dirtylimit_info; + size_t cpuinfo_idx = 0; + size_t i; + int ret = -1; + + if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VCPU_DIRTY_LIMIT)) + return 0; + + if (virDomainObjBeginJob(vm, VIR_JOB_MODIFY) < 0) + goto endjob; + + if (virDomainObjCheckActive(vm) < 0) + goto endjob; + + qemuDomainObjEnterMonitor(vm); + if (qemuMonitorQueryVcpuDirtyLimit(priv->mon, &dirtylimit_info) < 0) { + qemuDomainObjExitMonitor(vm); + goto endjob; + } + qemuDomainObjExitMonitor(vm); + + while (cpuinfo_idx < maxinfo) { + virVcpuInfoPtr vcpuinfo = info + cpuinfo_idx; + for (i = 0; i < dirtylimit_info.nvcpus && + i < virDomainDefGetVcpusMax(vm->def); i++) { + /* skip the offline virtual CPU */ + virDomainVcpuDef *vcpu = virDomainDefGetVcpu(vm->def, i); + if (!vcpu->online) + continue; + + /* match the index of virtual CPU */ + if (vcpuinfo->number == dirtylimit_info.limits[i].idx) { + vcpuinfo->current = dirtylimit_info.limits[i].current; + vcpuinfo->limit = dirtylimit_info.limits[i].limit; + } + } + cpuinfo_idx++; + } + ret = 0; + + endjob: + virDomainObjEndJob(vm); + return ret; +} + static int qemuDomainGetVcpus(virDomainPtr dom, virVcpuInfoPtr info, @@ -4623,6 +4674,10 @@ qemuDomainGetVcpus(virDomainPtr dom, ret = qemuDomainHelperGetVcpus(vm, info, NULL, NULL, maxinfo, cpumaps, maplen); + /* append dirty limit data to vcpu info */ + if (qemuDomainGetVcpuDirtyLimit(vm, info, maxinfo) < 0) + goto cleanup; + cleanup: virDomainObjEndAPI(&vm); return ret; -- 2.38.5

On Sat, Aug 13, 2022 at 11:06:27 +0800, ~hyman wrote:
From: Hyman Huang(黄勇) <yong.huang@smartx.com>
Extend qemuDomainGetVcpus for getting dirty page rate upper limit info so 'virsh vcpuinfo' api can display it.
Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- include/libvirt/libvirt-domain.h | 2 ++ src/qemu/qemu_driver.c | 55 ++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+)
diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index df7deffaa9..4c63d0be7c 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -2395,6 +2395,8 @@ struct _virVcpuInfo { int state; /* value from virVcpuState */ unsigned long long cpuTime; /* CPU time used, in nanoseconds */ int cpu; /* real CPU number, or one of the values from virVcpuHostCpuState */ + unsigned long long limit; /* virtual cpu dirty page rate upper limit in MB/s */ + unsigned long long current; /* current virtual cpu dirty page rate in MB/s */ };
Adding fields to C structs makes them ABI (binary) incompatible with programs compiled with libvirt before this change. Same goes for the RPC protocol. Thus we must never add to existing structs. I suggest you use the bulk stats API (virConnectGetAllDomainStats/virDomainListGetStats) which is extensible. NACK to this patch.

From: Hyman Huang(黄勇) <yong.huang@smartx.com> Implement qemuMonitorQueryVcpuDirtyLimit which query vcpu dirty limit info by calling qmp 'query-vcpu-dirty-limit'. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- src/qemu/qemu_monitor.c | 12 +++++++ src/qemu/qemu_monitor.h | 17 ++++++++++ src/qemu/qemu_monitor_json.c | 64 ++++++++++++++++++++++++++++++++++++ src/qemu/qemu_monitor_json.h | 4 +++ 4 files changed, 97 insertions(+) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 87926edec6..a8a6a6e4e2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -4518,3 +4518,15 @@ qemuMonitorSetVcpuDirtyLimit(qemuMonitor *mon, return qemuMonitorJSONSetVcpuDirtyLimit(mon, vcpu, rate); } + + +int +qemuMonitorQueryVcpuDirtyLimit(qemuMonitor *mon, + qemuMonitorVcpuDirtyLimitInfo *info) +{ + VIR_DEBUG("info=%p", info); + + QEMU_CHECK_MONITOR(mon); + + return qemuMonitorJSONQueryVcpuDirtyLimit(mon, info); +} diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 07a05365cf..1828bb202a 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -1584,3 +1584,20 @@ int qemuMonitorSetVcpuDirtyLimit(qemuMonitor *mon, int vcpu, unsigned long long rate); + +typedef struct _qemuMonitorVcpuDirtyLimit qemuMonitorVcpuDirtyLimit; +struct _qemuMonitorVcpuDirtyLimit { + int idx; /* virtual cpu index */ + unsigned long long limit; /* virtual cpu dirty page rate limit in MB/s */ + unsigned long long current; /* virtual cpu dirty page rate in MB/s */ +}; + +typedef struct _qemuMonitorVcpuDirtyLimitInfo qemuMonitorVcpuDirtyLimitInfo; +struct _qemuMonitorVcpuDirtyLimitInfo { + size_t nvcpus; /* number of virtual cpu */ + qemuMonitorVcpuDirtyLimit *limits; /* array of dirty page rate limit */ +}; + +int +qemuMonitorQueryVcpuDirtyLimit(qemuMonitor *mon, + qemuMonitorVcpuDirtyLimitInfo *info); diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 732366ab29..f8406c1857 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -8934,3 +8934,67 @@ qemuMonitorJSONSetVcpuDirtyLimit(qemuMonitor *mon, return 0; } + +static int +qemuMonitorJSONExtractVcpuDirtyLimitInfo(virJSONValue *data, + qemuMonitorVcpuDirtyLimitInfo *info) +{ + size_t nvcpus; + size_t i; + + nvcpus = virJSONValueArraySize(data); + info->nvcpus = nvcpus; + info->limits = g_new0(qemuMonitorVcpuDirtyLimit, nvcpus); + + for (i = 0; i < nvcpus; i++) { + virJSONValue *entry = virJSONValueArrayGet(data, i); + if (virJSONValueObjectGetNumberInt(entry, "cpu-index", + &info->limits[i].idx) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-vcpu-dirty-limit reply was missing 'cpu-index' data")); + return -1; + } + + if (virJSONValueObjectGetNumberUlong(entry, "limit-rate", + &info->limits[i].limit) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-vcpu-dirty-limit reply was missing 'limit-rate' data")); + return -1; + } + + if (virJSONValueObjectGetNumberUlong(entry, "current-rate", + &info->limits[i].current) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-vcpu-dirty-limit reply was missing 'current-rate' data")); + return -1; + } + } + + return 0; +} + +int +qemuMonitorJSONQueryVcpuDirtyLimit(qemuMonitor *mon, + qemuMonitorVcpuDirtyLimitInfo *info) +{ + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; + virJSONValue *data = NULL; + + if (!(cmd = qemuMonitorJSONMakeCommand("query-vcpu-dirty-limit", NULL))) + return -1; + + if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) + return -1; + + if (qemuMonitorJSONCheckError(cmd, reply) < 0) + return -1; + + if (!(data = virJSONValueObjectGetArray(reply, "return"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-vcpu-dirty-limit reply was missing 'return' data")); + return -1; + } + + return qemuMonitorJSONExtractVcpuDirtyLimitInfo(data, info); +} diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h index 89f61b3052..bd8131508b 100644 --- a/src/qemu/qemu_monitor_json.h +++ b/src/qemu/qemu_monitor_json.h @@ -830,3 +830,7 @@ int qemuMonitorJSONSetVcpuDirtyLimit(qemuMonitor *mon, int vcpu, unsigned long long rate); + +int +qemuMonitorJSONQueryVcpuDirtyLimit(qemuMonitor *mon, + qemuMonitorVcpuDirtyLimitInfo *info); -- 2.38.5

From: Hyman Huang(黄勇) <yong.huang@smartx.com> New libvirt APIs for limiting dirty page rate are introduced. Document this change. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- NEWS.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/NEWS.rst b/NEWS.rst index 1de8314a61..76a569736c 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -16,6 +16,22 @@ v9.6.0 (unreleased) * **Removed features** * **New features** + * qemu: Add support for limiting dirty page rate + + New API ``virDomainSetVcpuDirtyLimit()`` and virsh command + ``limit-dirty-page-rate`` are added to set the dirty page rate upper + limit. + + New API ``virDomainCancelVcpuDirtyLimit()`` and virsh command option + ``limit-dirty-page-rate --cancel`` are added to cancel the dirty page + rate upper limit. + + Note that this feature requires dirty-ring size to be configured. + + * qemu: Add support for querying dirty page rate upper limit + + User can query the dirty page rate limit with the virsh command + ``vcpuinfo``. * **Improvements** -- 2.38.5

From: Hyman Huang(黄勇) <yong.huang@smartx.com> Introduce the virDomainCancelVcpuDirtyLimit API to cancel the upper limit of dirty page rate. Signed-off-by: Hyman Huang(黄勇) <yong.huang@smartx.com> --- include/libvirt/libvirt-domain.h | 4 +++ src/driver-hypervisor.h | 6 ++++ src/libvirt-domain.c | 52 ++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 1 + src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 13 +++++++- src/remote_protocol-structs | 6 ++++ 7 files changed, 82 insertions(+), 1 deletion(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 4c63d0be7c..ddc4339c7e 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -6524,4 +6524,8 @@ int virDomainSetVcpuDirtyLimit(virDomainPtr domain, int vcpu, unsigned long long rate, unsigned int flags); + +int virDomainCancelVcpuDirtyLimit(virDomainPtr domain, + int vcpu, + unsigned int flags); #endif /* LIBVIRT_DOMAIN_H */ diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index e61b9efca5..4cfd9d1cdc 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -1454,6 +1454,11 @@ typedef int unsigned long long rate, unsigned int flags); +typedef int +(*virDrvDomainCancelVcpuDirtyLimit)(virDomainPtr domain, + int vcpu, + unsigned int flags); + typedef struct _virHypervisorDriver virHypervisorDriver; /** @@ -1727,4 +1732,5 @@ struct _virHypervisorDriver { virDrvDomainStartDirtyRateCalc domainStartDirtyRateCalc; virDrvDomainFDAssociate domainFDAssociate; virDrvDomainSetVcpuDirtyLimit domainSetVcpuDirtyLimit; + virDrvDomainCancelVcpuDirtyLimit domainCancelVcpuDirtyLimit; }; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 878d2a6d8c..f122775248 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -14113,3 +14113,55 @@ virDomainSetVcpuDirtyLimit(virDomainPtr domain, virDispatchError(domain->conn); return -1; } + +/** + * virDomainCancelVcpuDirtyLimit: + * + * @domain: pointer to domain object + * @vcpu: mandatory parameter only if the specified index of the + * virtual CPU is cancelled; ignored otherwise. + * @flags: bitwise-OR of supported virDomainDirtyLimitFlags + * + * Dynamically cancel the dirty page rate upper limit of the virtual CPUs. + * + * Returns 0 in case of success, -1 in case of failure. + * + * Since: 9.6.0 + */ +int +virDomainCancelVcpuDirtyLimit(virDomainPtr domain, + int vcpu, + unsigned int flags) +{ + virConnectPtr conn; + + VIR_DOMAIN_DEBUG(domain, "cancel vcpu[%d] dirty page rate limit", vcpu); + + virCheckFlags(VIR_DOMAIN_DIRTYLIMIT_VCPU | + VIR_DOMAIN_DIRTYLIMIT_ALL, -1); + + virResetLastError(); + + virCheckDomainReturn(domain, -1); + conn = domain->conn; + + virCheckReadOnlyGoto(conn->flags, error); + + VIR_EXCLUSIVE_FLAGS_GOTO(VIR_DOMAIN_DIRTYLIMIT_VCPU, + VIR_DOMAIN_DIRTYLIMIT_ALL, + error); + + if (conn->driver->domainCancelVcpuDirtyLimit) { + int ret; + ret = conn->driver->domainCancelVcpuDirtyLimit(domain, vcpu, flags); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(domain->conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 6fc01b518f..a6a7980cbc 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -935,6 +935,7 @@ LIBVIRT_9.0.0 { LIBVIRT_9.6.0 { global: virDomainSetVcpuDirtyLimit; + virDomainCancelVcpuDirtyLimit; } LIBVIRT_9.0.0; # .... define new API here using predicted next version number .... diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index c03e9e862b..4782de395f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8129,6 +8129,7 @@ static virHypervisorDriver hypervisor_driver = { .domainSetLaunchSecurityState = remoteDomainSetLaunchSecurityState, /* 8.0.0 */ .domainFDAssociate = remoteDomainFDAssociate, /* 9.0.0 */ .domainSetVcpuDirtyLimit = remoteDomainSetVcpuDirtyLimit, /* 9.6.0 */ + .domainCancelVcpuDirtyLimit = remoteDomainCancelVcpuDirtyLimit, /* 9.6.0 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 633b9af74e..49f0b44e34 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3945,6 +3945,12 @@ struct remote_domain_set_vcpu_dirty_limit_args { unsigned int flags; }; +struct remote_domain_cancel_vcpu_dirty_limit_args { + remote_nonnull_domain dom; + int vcpu; + unsigned int flags; +}; + /*----- Protocol. -----*/ /* Define the program number, protocol version and procedure numbers here. */ @@ -6989,5 +6995,10 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_SET_VCPU_DIRTY_LIMIT = 444 + REMOTE_PROC_DOMAIN_SET_VCPU_DIRTY_LIMIT = 444, + /** + * @generate: both + * @acl: domain:write + */ + REMOTE_PROC_DOMAIN_CANCEL_VCPU_DIRTY_LIMIT = 445 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index f7543ec667..29963a8ab5 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -3279,6 +3279,11 @@ struct remote_domain_set_vcpu_dirty_limit_args { uint64_t rate; u_int flags; }; +struct remote_domain_cancel_vcpu_dirty_limit_args { + remote_nonnull_domain dom; + int vcpu; + u_int flags; +}; enum remote_procedure { REMOTE_PROC_CONNECT_OPEN = 1, REMOTE_PROC_CONNECT_CLOSE = 2, @@ -3724,4 +3729,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_ABORT_JOB_FLAGS = 442, REMOTE_PROC_DOMAIN_FD_ASSOCIATE = 443, REMOTE_PROC_DOMAIN_SET_VCPU_DIRTY_LIMIT = 444, + REMOTE_PROC_DOMAIN_CANCEL_VCPU_DIRTY_LIMIT = 445, }; -- 2.38.5
participants (3)
-
Peter Krempa
-
Yong Huang
-
~hyman