在 2021/12/6 16:28, Peter Xu 写道:
On Sat, Dec 04, 2021 at 08:00:19PM +0800, Hyman Huang wrote:
在 2021/12/3 20:34, Markus Armbruster 写道:
huang...@chinatelecom.cn writes:
From: Hyman Huang(黄勇) <huang...@chinatelecom.cn>
Implement dirtyrate calculation periodically basing on
dirty-ring and throttle vCPU until it reachs the quota
dirty page rate given by user.
Introduce qmp commands "vcpu-dirty-limit", "query-vcpu-dirty-limit"
to enable, disable, query dirty page limit for virtual CPU.
Meanwhile, introduce corresponding hmp commands "vcpu_dirty_limit",
"info vcpu_dirty_limit" so developers can play with them easier.
Signed-off-by: Hyman Huang(黄勇) <huang...@chinatelecom.cn>
[...]
I see you replaced the interface. Back to square one...
diff --git a/qapi/migration.json b/qapi/migration.json
index 3da8fdf..dc15b3f 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1872,6 +1872,54 @@
'current-rate': 'int64' } }
##
+# @vcpu-dirty-limit:
+#
+# Set or cancel the upper limit of dirty page rate for a virtual CPU.
+#
+# Requires KVM with accelerator property "dirty-ring-size" set.
+# A virtual CPU's dirty page rate is a measure of its memory load.
+# To observe dirty page rates, use @calc-dirty-rate.
+#
+# @cpu-index: index of virtual CPU.
+#
+# @enable: true to enable, false to disable.
+#
+# @dirty-rate: upper limit of dirty page rate for virtual CPU.
+#
+# Since: 7.0
+#
+# Example:
+# {"execute": "vcpu-dirty-limit"}
+# "arguments": { "cpu-index": 0,
+# "enable": true,
+# "dirty-rate": 200 } }
+#
+##
+{ 'command': 'vcpu-dirty-limit',
+ 'data': { 'cpu-index': 'int',
+ 'enable': 'bool',
+ 'dirty-rate': 'uint64'} }
When @enable is false, @dirty-rate makes no sense and is not used (I
checked the code), but users have to specify it anyway. That's bad
design.
Better: drop @enable, make @dirty-rate optional, present means enable,
absent means disable.
Uh, if we drop @enable, enabling dirty limit should be like:
vcpu-dirty-limit cpu-index=0 dirty-rate=1000
And disabling dirty limit like:
vcpu-dirty-limit cpu-index=0
For disabling case, there is no hint of disabling in command
"vcpu-dirty-limit".
How about make @dirty-rate optional, when enable dirty limit, it should
present, ignored otherwise?
Sounds good, I think we can make both "enable" and "dirty-rate" optional.
To turn it on we either use "enable=true,dirty-rate=XXX" or "dirty-rate=XXX" >
To turn it off we use "enable=false".
Indeed, this make things more convenient.
>>
+
+##
+# @query-vcpu-dirty-limit:
+#
+# Returns information about the virtual CPU dirty limit status.
+#
+# @cpu-index: index of the virtual CPU to query, if not specified, all
+# virtual CPUs will be queried.
+#
+# Since: 7.0
+#
+# Example:
+# {"execute": "query-vcpu-dirty-limit"}
+# "arguments": { "cpu-index": 0 } }
+#
+##
+{ 'command': 'query-vcpu-dirty-limit',
+ 'data': { '*cpu-index': 'int' },
+ 'returns': [ 'DirtyLimitInfo' ] }
Why would anyone ever want to specify @cpu-index? Output isn't that
large even if you have a few hundred CPUs.
Let's keep things simple and drop the parameter.
Ok, this make things simple.
I found that it'll be challenging for any human being to identify "whether
he/she has turned throttle off for all vcpus".. I think that could be useful
when we finally decided to cancel current migration.
That's question, how about adding an optional argument "global" and
making "cpu-index", "enable", "dirty-rate" all optional in
"vcpu-dirty-limit", keeping the "cpu-index" and "global" options
mutually exclusive?
{ 'command': 'vcpu-dirty-limit',
'data': { '*cpu-index': 'int',
'*global': 'bool'
'*enable': 'bool',
'*dirty-rate': 'uint64'} }
In the case of enabling all vcpu throttle:
Either use "global=true,enable=true,dirty-rate=XXX" or
"global=true,dirty-rate=XXX"
In the case of disabling all vcpu throttle:
use "global=true,enable=false,dirty-rate=XXX"
In other case, we pass the same option just like what we did for
specified vcpu throttle before.
I thought about adding a "global=on/off" flag, but instead can we just return
the vcpu info for the ones that enabled the per-vcpu throttling? For anyone
who wants to read all vcpu dirty information he/she can use calc-dirty-rate.
Ok, I'll pick up this advice next version.
Thanks,