diff mbox series

[v10,11/12] migration/dirtyrate: Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function

Message ID 1600237327-33618-12-git-send-email-zhengchuan@huawei.com
State Superseded
Headers show
Series *** A Method for evaluating dirty page rate *** | expand

Commit Message

Zheng Chuan Sept. 16, 2020, 6:22 a.m. UTC
Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called

Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
---
 migration/dirtyrate.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi/migration.json   | 50 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+)

Comments

Dr. David Alan Gilbert Sept. 17, 2020, 9:08 a.m. UTC | #1
* Chuan Zheng (zhengchuan@huawei.com) wrote:
> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called

> 

> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


> ---

>  migration/dirtyrate.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++

>  qapi/migration.json   | 50 +++++++++++++++++++++++++++++++++++++++++

>  2 files changed, 112 insertions(+)

> 

> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c

> index 2a1a7fa..06f455d 100644

> --- a/migration/dirtyrate.c

> +++ b/migration/dirtyrate.c

> @@ -61,6 +61,24 @@ static int dirtyrate_set_state(int *state, int old_state, int new_state)

>      }

>  }

>  

> +static struct DirtyRateInfo *query_dirty_rate_info(void)

> +{

> +    int64_t dirty_rate = DirtyStat.dirty_rate;

> +    struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo));

> +

> +    if (atomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) {

> +        info->dirty_rate = dirty_rate;

> +    } else {

> +        info->dirty_rate = -1;

> +    }

> +

> +    info->status = CalculatingState;

> +    info->start_time = DirtyStat.start_time;

> +    info->calc_time = DirtyStat.calc_time;

> +

> +    return info;

> +}

> +

>  static void reset_dirtyrate_stat(void)

>  {

>      DirtyStat.total_dirty_samples = 0;

> @@ -318,6 +336,8 @@ static void calculate_dirtyrate(struct DirtyRateConfig config)

>  

>      msec = config.sample_period_seconds * 1000;

>      msec = set_sample_page_period(msec, initial_time);

> +    DirtyStat.start_time = initial_time / 1000;

> +    DirtyStat.calc_time = msec / 1000;

>  

>      rcu_read_lock();

>      if (!compare_page_hash_info(block_dinfo, block_count)) {

> @@ -353,3 +373,45 @@ void *get_dirtyrate_thread(void *arg)

>      }

>      return NULL;

>  }

> +

> +void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)

> +{

> +    static struct DirtyRateConfig config;

> +    QemuThread thread;

> +    int ret;

> +

> +    /*

> +     * If the dirty rate is already being measured, don't attempt to start.

> +     */

> +    if (atomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURING) {

> +        error_setg(errp, "the dirty rate is already being measured.");

> +        return;

> +    }

> +

> +    if (!is_sample_period_valid(calc_time)) {

> +        error_setg(errp, "calc-time is out of range[%d, %d].",

> +                         MIN_FETCH_DIRTYRATE_TIME_SEC,

> +                         MAX_FETCH_DIRTYRATE_TIME_SEC);

> +        return;

> +    }

> +

> +    /*

> +     * Init calculation state as unstarted.

> +     */

> +    ret = dirtyrate_set_state(&CalculatingState, CalculatingState,

> +                              DIRTY_RATE_STATUS_UNSTARTED);

> +    if (ret == -1) {

> +        error_setg(errp, "init dirty rate calculation state failed.");

> +        return;

> +    }

> +

> +    config.sample_period_seconds = calc_time;

> +    config.sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;

> +    qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,

> +                       (void *)&config, QEMU_THREAD_DETACHED);

> +}

> +

> +struct DirtyRateInfo *qmp_query_dirty_rate(Error **errp)

> +{

> +    return query_dirty_rate_info();

> +}

> diff --git a/qapi/migration.json b/qapi/migration.json

> index 061ff25..4b980a0 100644

> --- a/qapi/migration.json

> +++ b/qapi/migration.json

> @@ -1737,3 +1737,53 @@

>  ##

>  { 'enum': 'DirtyRateStatus',

>    'data': [ 'unstarted', 'measuring', 'measured'] }

> +

> +##

> +# @DirtyRateInfo:

> +#

> +# Information about current dirty page rate of vm.

> +#

> +# @dirty-rate: @dirtyrate describing the dirty page rate of vm

> +#          in units of MB/s.

> +#          If this field return '-1', it means querying is not

> +#          start or not complete.

> +#

> +# @status: status containing dirtyrate query status includes

> +#          'unstarted' or 'measuring' or 'measured'

> +#

> +# @start-time: start time in units of second for calculation

> +#

> +# @calc-time: time in units of second for sample dirty pages

> +#

> +# Since: 5.2

> +#

> +##

> +{ 'struct': 'DirtyRateInfo',

> +  'data': {'dirty-rate': 'int64',

> +           'status': 'DirtyRateStatus',

> +           'start-time': 'int64',

> +           'calc-time': 'int64'} }

> +

> +##

> +# @calc-dirty-rate:

> +#

> +# start calculating dirty page rate for vm

> +#

> +# @calc-time: time in units of second for sample dirty pages

> +#

> +# Since: 5.2

> +#

> +# Example:

> +#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }

> +#

> +##

> +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }

> +

> +##

> +# @query-dirty-rate:

> +#

> +# query dirty page rate in units of MB/s for vm

> +#

> +# Since: 5.2

> +##

> +{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }

> -- 

> 1.8.3.1

> 

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Eric Blake Sept. 23, 2020, 6:17 p.m. UTC | #2
On 9/16/20 1:22 AM, Chuan Zheng wrote:
> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called
> 
> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>
> ---

> +++ b/qapi/migration.json
> @@ -1737,3 +1737,53 @@
>   ##
>   { 'enum': 'DirtyRateStatus',
>     'data': [ 'unstarted', 'measuring', 'measured'] }
> +
> +##
> +# @DirtyRateInfo:
> +#
> +# Information about current dirty page rate of vm.
> +#
> +# @dirty-rate: @dirtyrate describing the dirty page rate of vm
> +#          in units of MB/s.
> +#          If this field return '-1', it means querying is not
> +#          start or not complete.

Grammar:

it means querying has not yet started or completed.

Should this field instead be optional, and omitted for those cases?  In 
which case, I'd suggest:

...in units of MB/s, present only when querying the rate has completed.

> +#
> +# @status: status containing dirtyrate query status includes
> +#          'unstarted' or 'measuring' or 'measured'
> +#
> +# @start-time: start time in units of second for calculation
> +#
> +# @calc-time: time in units of second for sample dirty pages
> +#
> +# Since: 5.2
> +#
> +##
> +{ 'struct': 'DirtyRateInfo',
> +  'data': {'dirty-rate': 'int64',
> +           'status': 'DirtyRateStatus',
> +           'start-time': 'int64',
> +           'calc-time': 'int64'} }
> +
> +##
> +# @calc-dirty-rate:
> +#
> +# start calculating dirty page rate for vm
> +#
> +# @calc-time: time in units of second for sample dirty pages
> +#
> +# Since: 5.2
> +#
> +# Example:
> +#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }
> +#
> +##
> +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
> +
> +##
> +# @query-dirty-rate:
> +#
> +# query dirty page rate in units of MB/s for vm
> +#
> +# Since: 5.2
> +##
> +{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }
>
Dr. David Alan Gilbert Sept. 23, 2020, 7:03 p.m. UTC | #3
* Eric Blake (eblake@redhat.com) wrote:
> On 9/16/20 1:22 AM, Chuan Zheng wrote:

> > Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called

> > 

> > Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>

> > ---

> 

> > +++ b/qapi/migration.json

> > @@ -1737,3 +1737,53 @@

> >   ##

> >   { 'enum': 'DirtyRateStatus',

> >     'data': [ 'unstarted', 'measuring', 'measured'] }

> > +

> > +##

> > +# @DirtyRateInfo:

> > +#

> > +# Information about current dirty page rate of vm.

> > +#

> > +# @dirty-rate: @dirtyrate describing the dirty page rate of vm

> > +#          in units of MB/s.

> > +#          If this field return '-1', it means querying is not

> > +#          start or not complete.

> 

> Grammar:

> 

> it means querying has not yet started or completed.

> 

> Should this field instead be optional, and omitted for those cases?  In

> which case, I'd suggest:

> 

> ...in units of MB/s, present only when querying the rate has completed.


I've already got it queued; I'll fix up the grammar; if someone wants to
send a change to make it optional before this version freezes that's OK.

Dave

> 

> > +#

> > +# @status: status containing dirtyrate query status includes

> > +#          'unstarted' or 'measuring' or 'measured'

> > +#

> > +# @start-time: start time in units of second for calculation

> > +#

> > +# @calc-time: time in units of second for sample dirty pages

> > +#

> > +# Since: 5.2

> > +#

> > +##

> > +{ 'struct': 'DirtyRateInfo',

> > +  'data': {'dirty-rate': 'int64',

> > +           'status': 'DirtyRateStatus',

> > +           'start-time': 'int64',

> > +           'calc-time': 'int64'} }

> > +

> > +##

> > +# @calc-dirty-rate:

> > +#

> > +# start calculating dirty page rate for vm

> > +#

> > +# @calc-time: time in units of second for sample dirty pages

> > +#

> > +# Since: 5.2

> > +#

> > +# Example:

> > +#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }

> > +#

> > +##

> > +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }

> > +

> > +##

> > +# @query-dirty-rate:

> > +#

> > +# query dirty page rate in units of MB/s for vm

> > +#

> > +# Since: 5.2

> > +##

> > +{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }

> > 

> 

> -- 

> Eric Blake, Principal Software Engineer

> Red Hat, Inc.           +1-919-301-3226

> Virtualization:  qemu.org | libvirt.org

-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Zheng Chuan Sept. 24, 2020, 8:42 a.m. UTC | #4
On 2020/9/24 3:03, Dr. David Alan Gilbert wrote:
> * Eric Blake (eblake@redhat.com) wrote:

>> On 9/16/20 1:22 AM, Chuan Zheng wrote:

>>> Implement qmp_cal_dirty_rate()/qmp_get_dirty_rate() function which could be called

>>>

>>> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com>

>>> ---

>>

>>> +++ b/qapi/migration.json

>>> @@ -1737,3 +1737,53 @@

>>>   ##

>>>   { 'enum': 'DirtyRateStatus',

>>>     'data': [ 'unstarted', 'measuring', 'measured'] }

>>> +

>>> +##

>>> +# @DirtyRateInfo:

>>> +#

>>> +# Information about current dirty page rate of vm.

>>> +#

>>> +# @dirty-rate: @dirtyrate describing the dirty page rate of vm

>>> +#          in units of MB/s.

>>> +#          If this field return '-1', it means querying is not

>>> +#          start or not complete.

>>

>> Grammar:

>>

>> it means querying has not yet started or completed.

>>

>> Should this field instead be optional, and omitted for those cases?  In

>> which case, I'd suggest:

>>

>> ...in units of MB/s, present only when querying the rate has completed.

> 

Hi, Eric.
Thanks for your review.
Yeah, it could be optional.
and should it need keep start-time and calc-time when omit dirtyrate?
like:
{"return":{"status":"measuring","start-time":3718293,"calc-time":1},"id":"libvirt-15"}
or
{"return":{"status":"unstarted","start-time":3718293,"calc-time":1},"id":"libvirt-15"}

> I've already got it queued; I'll fix up the grammar; if someone wants to

> send a change to make it optional before this version freezes that's OK.

> 

> Dave

> 

>>

>>> +#

>>> +# @status: status containing dirtyrate query status includes

>>> +#          'unstarted' or 'measuring' or 'measured'

>>> +#

>>> +# @start-time: start time in units of second for calculation

>>> +#

>>> +# @calc-time: time in units of second for sample dirty pages

>>> +#

>>> +# Since: 5.2

>>> +#

>>> +##

>>> +{ 'struct': 'DirtyRateInfo',

>>> +  'data': {'dirty-rate': 'int64',

>>> +           'status': 'DirtyRateStatus',

>>> +           'start-time': 'int64',

>>> +           'calc-time': 'int64'} }

>>> +

>>> +##

>>> +# @calc-dirty-rate:

>>> +#

>>> +# start calculating dirty page rate for vm

>>> +#

>>> +# @calc-time: time in units of second for sample dirty pages

>>> +#

>>> +# Since: 5.2

>>> +#

>>> +# Example:

>>> +#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }

>>> +#

>>> +##

>>> +{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }

>>> +

>>> +##

>>> +# @query-dirty-rate:

>>> +#

>>> +# query dirty page rate in units of MB/s for vm

>>> +#

>>> +# Since: 5.2

>>> +##

>>> +{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }

>>>

>>

>> -- 

>> Eric Blake, Principal Software Engineer

>> Red Hat, Inc.           +1-919-301-3226

>> Virtualization:  qemu.org | libvirt.org
diff mbox series

Patch

diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c
index 2a1a7fa..06f455d 100644
--- a/migration/dirtyrate.c
+++ b/migration/dirtyrate.c
@@ -61,6 +61,24 @@  static int dirtyrate_set_state(int *state, int old_state, int new_state)
     }
 }
 
+static struct DirtyRateInfo *query_dirty_rate_info(void)
+{
+    int64_t dirty_rate = DirtyStat.dirty_rate;
+    struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo));
+
+    if (atomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) {
+        info->dirty_rate = dirty_rate;
+    } else {
+        info->dirty_rate = -1;
+    }
+
+    info->status = CalculatingState;
+    info->start_time = DirtyStat.start_time;
+    info->calc_time = DirtyStat.calc_time;
+
+    return info;
+}
+
 static void reset_dirtyrate_stat(void)
 {
     DirtyStat.total_dirty_samples = 0;
@@ -318,6 +336,8 @@  static void calculate_dirtyrate(struct DirtyRateConfig config)
 
     msec = config.sample_period_seconds * 1000;
     msec = set_sample_page_period(msec, initial_time);
+    DirtyStat.start_time = initial_time / 1000;
+    DirtyStat.calc_time = msec / 1000;
 
     rcu_read_lock();
     if (!compare_page_hash_info(block_dinfo, block_count)) {
@@ -353,3 +373,45 @@  void *get_dirtyrate_thread(void *arg)
     }
     return NULL;
 }
+
+void qmp_calc_dirty_rate(int64_t calc_time, Error **errp)
+{
+    static struct DirtyRateConfig config;
+    QemuThread thread;
+    int ret;
+
+    /*
+     * If the dirty rate is already being measured, don't attempt to start.
+     */
+    if (atomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURING) {
+        error_setg(errp, "the dirty rate is already being measured.");
+        return;
+    }
+
+    if (!is_sample_period_valid(calc_time)) {
+        error_setg(errp, "calc-time is out of range[%d, %d].",
+                         MIN_FETCH_DIRTYRATE_TIME_SEC,
+                         MAX_FETCH_DIRTYRATE_TIME_SEC);
+        return;
+    }
+
+    /*
+     * Init calculation state as unstarted.
+     */
+    ret = dirtyrate_set_state(&CalculatingState, CalculatingState,
+                              DIRTY_RATE_STATUS_UNSTARTED);
+    if (ret == -1) {
+        error_setg(errp, "init dirty rate calculation state failed.");
+        return;
+    }
+
+    config.sample_period_seconds = calc_time;
+    config.sample_pages_per_gigabytes = DIRTYRATE_DEFAULT_SAMPLE_PAGES;
+    qemu_thread_create(&thread, "get_dirtyrate", get_dirtyrate_thread,
+                       (void *)&config, QEMU_THREAD_DETACHED);
+}
+
+struct DirtyRateInfo *qmp_query_dirty_rate(Error **errp)
+{
+    return query_dirty_rate_info();
+}
diff --git a/qapi/migration.json b/qapi/migration.json
index 061ff25..4b980a0 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1737,3 +1737,53 @@ 
 ##
 { 'enum': 'DirtyRateStatus',
   'data': [ 'unstarted', 'measuring', 'measured'] }
+
+##
+# @DirtyRateInfo:
+#
+# Information about current dirty page rate of vm.
+#
+# @dirty-rate: @dirtyrate describing the dirty page rate of vm
+#          in units of MB/s.
+#          If this field return '-1', it means querying is not
+#          start or not complete.
+#
+# @status: status containing dirtyrate query status includes
+#          'unstarted' or 'measuring' or 'measured'
+#
+# @start-time: start time in units of second for calculation
+#
+# @calc-time: time in units of second for sample dirty pages
+#
+# Since: 5.2
+#
+##
+{ 'struct': 'DirtyRateInfo',
+  'data': {'dirty-rate': 'int64',
+           'status': 'DirtyRateStatus',
+           'start-time': 'int64',
+           'calc-time': 'int64'} }
+
+##
+# @calc-dirty-rate:
+#
+# start calculating dirty page rate for vm
+#
+# @calc-time: time in units of second for sample dirty pages
+#
+# Since: 5.2
+#
+# Example:
+#   {"command": "calc-dirty-rate", "data": {"calc-time": 1} }
+#
+##
+{ 'command': 'calc-dirty-rate', 'data': {'calc-time': 'int64'} }
+
+##
+# @query-dirty-rate:
+#
+# query dirty page rate in units of MB/s for vm
+#
+# Since: 5.2
+##
+{ 'command': 'query-dirty-rate', 'returns': 'DirtyRateInfo' }