From: Zheng Chuan <zhengchuan@huawei.com>
Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
by libvirt api.
Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
---
migration/Makefile.objs | 1 +
migration/dirtyrate.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
qapi/migration.json | 24 ++++++++++++++++++++++++
qapi/pragma.json | 3 ++-
4 files changed, 72 insertions(+), 1 deletion(-)
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 0fc619e..12ae98c 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -6,6 +6,7 @@ common-obj-y += qemu-file.o global_state.o
common-obj-y += qemu-file-channel.o
common-obj-y += xbzrle.o postcopy-ram.o
common-obj-y += qjson.o
+common-obj-y += dirtyrate.o
common-obj-y += block-dirty-bitmap.o
common-obj-y += multifd.o
common-obj-y += multifd-zlib.o
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index d87e16d..5717521 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -31,6 +31,21 @@ CalculatingDirtyRateStage calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
INTERNAL_RAMBLOCK_FOREACH(block) \
if (!qemu_ram_is_migratable(block)) {} else
+static int64_t get_dirty_rate_info(void)
+{
+ int64_t dirty_rate = dirty_stat.dirty_rate;
+ /*
+ * Return dirty_rate only when we get CAL_DIRTY_RATE_END.
+ * And we must initial calculating_dirty_rate_stage.
+ */
+ if (calculating_dirty_rate_stage == CAL_DIRTY_RATE_END) {
+ calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
+ return dirty_rate;
+ } else {
+ return -1;
+ }
+}
+
static void reset_dirtyrate_stat(void)
{
dirty_stat.total_dirty_samples = 0;
@@ -377,3 +392,33 @@ void *get_dirtyrate_thread(void *arg)
return NULL;
}
+
+void qmp_cal_dirty_rate(int64_t value, Error **errp)
+{
+ static struct dirtyrate_config dirtyrate_config;
+ QemuThread thread;
+
+ /*
+ * We don't begin calculating thread only when it's in calculating status.
+ */
+ if (calculating_dirty_rate_stage == CAL_DIRTY_RATE_ING) {
+ return;
+ }
+
+ /*
+ * If we get CAL_DIRTY_RATE_END here, libvirtd may be restarted at last round,
+ * we need to initial it.
+ */
+ if (calculating_dirty_rate_stage == CAL_DIRTY_RATE_END)
+ calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
+
+ dirtyrate_config.sample_period_seconds = value;
+ dirtyrate_config.sample_pages_per_gigabytes = sample_pages_per_gigabytes;
+ qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
+ (void *)&dirtyrate_config, QEMU_THREAD_DETACHED);
+}
+
+int64_t qmp_get_dirty_rate(Error **errp)
+{
+ return get_dirty_rate_info();
+}
diff --git a/qapi/migration.json b/qapi/migration.json
index d500055..59f35bb 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1621,3 +1621,27 @@
##
{ 'event': 'UNPLUG_PRIMARY',
'data': { 'device-id': 'str' } }
+
+##
+# @cal_dirty_rate:
+#
+# start calculating dirty rate for vm
+#
+# @value: time for sample dirty pages
+#
+# Since: 5.1
+#
+# Example:
+# {"command": "cal_dirty_rate", "data": {"value": 1} }
+#
+##
+{ 'command': 'cal_dirty_rate', 'data': {'value': 'int64'} }
+
+##
+# @get_dirty_rate:
+#
+# get dirty rate for vm
+#
+# Since: 5.1
+##
+{ 'command': 'get_dirty_rate', 'returns': 'int64' }
diff --git a/qapi/pragma.json b/qapi/pragma.json
index cffae27..ecd294b 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -10,7 +10,8 @@
'query-migrate-cache-size',
'query-tpm-models',
'query-tpm-types',
- 'ringbuf-read' ],
+ 'ringbuf-read',
+ 'get_dirty_rate' ],
'name-case-whitelist': [
'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
'CpuInfoMIPS', # PC, visible through query-cpu
--
1.8.3.1
On 7/24/20 10:11 PM, Chuan Zheng wrote:
> From: Zheng Chuan <zhengchuan@huawei.com>
>
> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
> by libvirt api.
>
> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
> ---
> +++ b/qapi/migration.json
> @@ -1621,3 +1621,27 @@
> ##
> { 'event': 'UNPLUG_PRIMARY',
> 'data': { 'device-id': 'str' } }
> +
> +##
> +# @cal_dirty_rate:
New QMP commands should be named favoring '-' over '_'; also, it doesn't
hurt to spell it out:
calculate-dirty-rate
> +#
> +# start calculating dirty rate for vm
> +#
> +# @value: time for sample dirty pages
In what unit?
> +#
> +# Since: 5.1
We've missed 5.1; this will need to be updated to 5.2.
> +#
> +# Example:
> +# {"command": "cal_dirty_rate", "data": {"value": 1} }
> +#
> +##
> +{ 'command': 'cal_dirty_rate', 'data': {'value': 'int64'} }
> +
> +##
> +# @get_dirty_rate:
get-dirty-rate, except that we tend to use 'query-' as the prefix for
commands that read values.
> +#
> +# get dirty rate for vm
> +#
> +# Since: 5.1
5.2
What units is the rate expressed in?
> +##
> +{ 'command': 'get_dirty_rate', 'returns': 'int64' }
> diff --git a/qapi/pragma.json b/qapi/pragma.json
> index cffae27..ecd294b 100644
> --- a/qapi/pragma.json
> +++ b/qapi/pragma.json
> @@ -10,7 +10,8 @@
> 'query-migrate-cache-size',
> 'query-tpm-models',
> 'query-tpm-types',
> - 'ringbuf-read' ],
> + 'ringbuf-read',
> + 'get_dirty_rate' ],
> 'name-case-whitelist': [
> 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
> 'CpuInfoMIPS', # PC, visible through query-cpu
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
On 2020/8/5 0:28, Eric Blake wrote:
> On 7/24/20 10:11 PM, Chuan Zheng wrote:
>> From: Zheng Chuan <zhengchuan@huawei.com>
>>
>> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
>> by libvirt api.
>>
>> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
>> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
>> ---
>
>> +++ b/qapi/migration.json
>> @@ -1621,3 +1621,27 @@
>> ##
>> { 'event': 'UNPLUG_PRIMARY',
>> 'data': { 'device-id': 'str' } }
>> +
>> +##
>> +# @cal_dirty_rate:
>
> New QMP commands should be named favoring '-' over '_'; also, it doesn't hurt to spell it out:
>
> calculate-dirty-rate
>
Hi, Eric.
Thank you for your review.
I'll fix that in V2.
>> +#
>> +# start calculating dirty rate for vm
>> +#
>> +# @value: time for sample dirty pages
>
> In what unit?
>
The unit is second, i'll make comments in V2:)
>> +#
>> +# Since: 5.1
>
> We've missed 5.1; this will need to be updated to 5.2.
>
>> +#
>> +# Example:
>> +# {"command": "cal_dirty_rate", "data": {"value": 1} }
>> +#
>> +##
>> +{ 'command': 'cal_dirty_rate', 'data': {'value': 'int64'} }
>> +
>> +##
>> +# @get_dirty_rate:
>
> get-dirty-rate, except that we tend to use 'query-' as the prefix for commands that read values.
>
>> +#
>> +# get dirty rate for vm
>> +#
>> +# Since: 5.1
>
> 5.2
>
> What units is the rate expressed in?
>
The unit is MB/s, i'll make comments in V2:)
>
>> +##
>> +{ 'command': 'get_dirty_rate', 'returns': 'int64' }
>> diff --git a/qapi/pragma.json b/qapi/pragma.json
>> index cffae27..ecd294b 100644
>> --- a/qapi/pragma.json
>> +++ b/qapi/pragma.json
>> @@ -10,7 +10,8 @@
>> 'query-migrate-cache-size',
>> 'query-tpm-models',
>> 'query-tpm-types',
>> - 'ringbuf-read' ],
>> + 'ringbuf-read',
>> + 'get_dirty_rate' ],
>> 'name-case-whitelist': [
>> 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
>> 'CpuInfoMIPS', # PC, visible through query-cpu
>>
>
On 7/24/20 10:11 PM, Chuan Zheng wrote:
> From: Zheng Chuan <zhengchuan@huawei.com>
>
> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
> by libvirt api.
>
> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
> +##
> +{ 'command': 'get_dirty_rate', 'returns': 'int64' }
> diff --git a/qapi/pragma.json b/qapi/pragma.json
> index cffae27..ecd294b 100644
> --- a/qapi/pragma.json
> +++ b/qapi/pragma.json
> @@ -10,7 +10,8 @@
> 'query-migrate-cache-size',
> 'query-tpm-models',
> 'query-tpm-types',
> - 'ringbuf-read' ],
> + 'ringbuf-read',
> + 'get_dirty_rate' ],
Nack. You should not have to change the whitelist; this is evidence
that your command is returning the wrong type. Instead, you should be
using:
{ 'command': 'get-dirty-rate', 'returns': { 'rate': 'int64' } }
and populating a struct, so that if we ever want to return more than
just a single rate, we can extend the command in-place by adding to the
struct.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
* Chuan Zheng (zhengchuan@huawei.com) wrote:
> From: Zheng Chuan <zhengchuan@huawei.com>
>
> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
> by libvirt api.
>
> Signed-off-by: Zheng Chuan <zhengchuan@huawei.com>
> Signed-off-by: YanYing Zhang <ann.zhuangyanying@huawei.com>
> ---
> migration/Makefile.objs | 1 +
> migration/dirtyrate.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> qapi/migration.json | 24 ++++++++++++++++++++++++
> qapi/pragma.json | 3 ++-
> 4 files changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/migration/Makefile.objs b/migration/Makefile.objs
> index 0fc619e..12ae98c 100644
> --- a/migration/Makefile.objs
> +++ b/migration/Makefile.objs
> @@ -6,6 +6,7 @@ common-obj-y += qemu-file.o global_state.o
> common-obj-y += qemu-file-channel.o
> common-obj-y += xbzrle.o postcopy-ram.o
> common-obj-y += qjson.o
> +common-obj-y += dirtyrate.o
> common-obj-y += block-dirty-bitmap.o
> common-obj-y += multifd.o
> common-obj-y += multifd-zlib.o
> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
> index d87e16d..5717521 100644
> --- a/migration/dirtyrate.c
> +++ b/migration/dirtyrate.c
> @@ -31,6 +31,21 @@ CalculatingDirtyRateStage calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
> INTERNAL_RAMBLOCK_FOREACH(block) \
> if (!qemu_ram_is_migratable(block)) {} else
>
> +static int64_t get_dirty_rate_info(void)
> +{
> + int64_t dirty_rate = dirty_stat.dirty_rate;
> + /*
> + * Return dirty_rate only when we get CAL_DIRTY_RATE_END.
> + * And we must initial calculating_dirty_rate_stage.
> + */
> + if (calculating_dirty_rate_stage == CAL_DIRTY_RATE_END) {
> + calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
> + return dirty_rate;
I don't think a 'get' should change state - in particular I think we
should be able to call 'get' multiple times and get the result back.
Also, instead of just returning -1 you should probably give an error;
e.g. 'Still measuring' or 'not yet measured'
> + } else {
> + return -1;
> + }
> +}
> +
> static void reset_dirtyrate_stat(void)
> {
> dirty_stat.total_dirty_samples = 0;
> @@ -377,3 +392,33 @@ void *get_dirtyrate_thread(void *arg)
>
> return NULL;
> }
> +
> +void qmp_cal_dirty_rate(int64_t value, Error **errp)
> +{
> + static struct dirtyrate_config dirtyrate_config;
> + QemuThread thread;
> +
> + /*
> + * We don't begin calculating thread only when it's in calculating status.
> + */
> + if (calculating_dirty_rate_stage == CAL_DIRTY_RATE_ING) {
> + return;
> + }
> +
> + /*
> + * If we get CAL_DIRTY_RATE_END here, libvirtd may be restarted at last round,
> + * we need to initial it.
> + */
> + if (calculating_dirty_rate_stage == CAL_DIRTY_RATE_END)
> + calculating_dirty_rate_stage = CAL_DIRTY_RATE_INIT;
Note please run the patches through scripts/checkpatch.pl - you need
some {'s here and that script tells you about most of these things.
> + dirtyrate_config.sample_period_seconds = value;
Ah, this is where the config comes from - OK, I think you said there was
a max, so you should probably check that the maximum is sensible here
and give an error if it's not.
Dave
> + dirtyrate_config.sample_pages_per_gigabytes = sample_pages_per_gigabytes;
> + qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
> + (void *)&dirtyrate_config, QEMU_THREAD_DETACHED);
> +}
> +
> +int64_t qmp_get_dirty_rate(Error **errp)
> +{
> + return get_dirty_rate_info();
> +}
> diff --git a/qapi/migration.json b/qapi/migration.json
> index d500055..59f35bb 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1621,3 +1621,27 @@
> ##
> { 'event': 'UNPLUG_PRIMARY',
> 'data': { 'device-id': 'str' } }
> +
> +##
> +# @cal_dirty_rate:
> +#
> +# start calculating dirty rate for vm
> +#
> +# @value: time for sample dirty pages
> +#
> +# Since: 5.1
> +#
> +# Example:
> +# {"command": "cal_dirty_rate", "data": {"value": 1} }
> +#
> +##
> +{ 'command': 'cal_dirty_rate', 'data': {'value': 'int64'} }
I'm OK with abbreviating 'calculate' but prefer calc.
> +##
> +# @get_dirty_rate:
> +#
> +# get dirty rate for vm
> +#
> +# Since: 5.1
> +##
> +{ 'command': 'get_dirty_rate', 'returns': 'int64' }
> diff --git a/qapi/pragma.json b/qapi/pragma.json
> index cffae27..ecd294b 100644
> --- a/qapi/pragma.json
> +++ b/qapi/pragma.json
> @@ -10,7 +10,8 @@
> 'query-migrate-cache-size',
> 'query-tpm-models',
> 'query-tpm-types',
> - 'ringbuf-read' ],
> + 'ringbuf-read',
> + 'get_dirty_rate' ],
> 'name-case-whitelist': [
> 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status
> 'CpuInfoMIPS', # PC, visible through query-cpu
> --
> 1.8.3.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
© 2016 - 2025 Red Hat, Inc.