| Message ID | 1601182520-130450-1-git-send-email-zhengchuan@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration/dirtyrate: present dirty rate only when querying the rate has completed | expand |
On Sunday, 2020-09-27 at 12:55:20 +08, Chuan Zheng wrote: > Make dirty_rate field optional, present dirty rate only when querying > the rate has completed. > The qmp results is shown as follow: > @unstarted: > {"return":{"status":"unstarted","start-time":0,"calc-time":0},"id":"libvirt-12"} > @measuring: > {"return":{"status":"measuring","start-time":0,"calc-time":0},"id":"libvirt-14"} Not this patch, but the "measuring" state could include both the start-time and the calc-time, allowing a caller to determine when they should expect to come back looking for a result. > @measured: > {"return":{"status":"measured","dirty-rate":4,"start-time":150146,"calc-time":1},"id":"libvirt-15"} > > Signed-off-by: Chuan Zheng <zhengchuan@huawei.com> With the minor wording changes below... Reviewed-by: David Edmondson <david.edmondson@oracle.com> > --- > migration/dirtyrate.c | 3 +-- > qapi/migration.json | 9 ++++----- > 2 files changed, 5 insertions(+), 7 deletions(-) > > diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c > index 68577ef..9024b0f 100644 > --- a/migration/dirtyrate.c > +++ b/migration/dirtyrate.c > @@ -69,9 +69,8 @@ static struct DirtyRateInfo *query_dirty_rate_info(void) > struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo)); > > if (qatomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) { > + info->has_dirty_rate = true; > info->dirty_rate = dirty_rate; > - } else { > - info->dirty_rate = -1; > } > > info->status = CalculatingState; > diff --git a/qapi/migration.json b/qapi/migration.json > index ce2216c..6e428f7 100644 > --- a/qapi/migration.json > +++ b/qapi/migration.json > @@ -1743,10 +1743,9 @@ > # > # 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 returns '-1', it means querying has not > -# yet started or completed. > +# @dirty-rate: dirty-rate describing the dirty page rate of vm > +# in units of MB/s, present only when querying the > +# rate has completed. How about: @dirty-rate: an estimate of the dirty page rate of the VM in units of MB/s, present only when estimating the rate has completed. > # > # @status: status containing dirtyrate query status includes > # 'unstarted' or 'measuring' or 'measured' > @@ -1759,7 +1758,7 @@ > # > ## > { 'struct': 'DirtyRateInfo', > - 'data': {'dirty-rate': 'int64', > + 'data': {'*dirty-rate': 'int64', > 'status': 'DirtyRateStatus', > 'start-time': 'int64', > 'calc-time': 'int64'} } > -- > 1.8.3.1 dme. -- I used to worry, thought I was goin' mad in a hurry.
On 2020/9/28 20:23, David Edmondson wrote: > On Sunday, 2020-09-27 at 12:55:20 +08, Chuan Zheng wrote: > >> Make dirty_rate field optional, present dirty rate only when querying >> the rate has completed. >> The qmp results is shown as follow: >> @unstarted: >> {"return":{"status":"unstarted","start-time":0,"calc-time":0},"id":"libvirt-12"} >> @measuring: >> {"return":{"status":"measuring","start-time":0,"calc-time":0},"id":"libvirt-14"} > > Not this patch, but the "measuring" state could include both the > start-time and the calc-time, allowing a caller to determine when they > should expect to come back looking for a result. > Hi, David. Yes, maybe we should record start-time and calc-time in reset_dirtyrate_stat() and move it forward just after setting CalculatingState status succeed in get_dirtyrate_thread. >> @measured: >> {"return":{"status":"measured","dirty-rate":4,"start-time":150146,"calc-time":1},"id":"libvirt-15"} >> >> Signed-off-by: Chuan Zheng <zhengchuan@huawei.com> > > With the minor wording changes below... > > Reviewed-by: David Edmondson <david.edmondson@oracle.com> > >> --- >> migration/dirtyrate.c | 3 +-- >> qapi/migration.json | 9 ++++----- >> 2 files changed, 5 insertions(+), 7 deletions(-) >> >> diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c >> index 68577ef..9024b0f 100644 >> --- a/migration/dirtyrate.c >> +++ b/migration/dirtyrate.c >> @@ -69,9 +69,8 @@ static struct DirtyRateInfo *query_dirty_rate_info(void) >> struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo)); >> >> if (qatomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) { >> + info->has_dirty_rate = true; >> info->dirty_rate = dirty_rate; >> - } else { >> - info->dirty_rate = -1; >> } >> >> info->status = CalculatingState; >> diff --git a/qapi/migration.json b/qapi/migration.json >> index ce2216c..6e428f7 100644 >> --- a/qapi/migration.json >> +++ b/qapi/migration.json >> @@ -1743,10 +1743,9 @@ >> # >> # 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 returns '-1', it means querying has not >> -# yet started or completed. >> +# @dirty-rate: dirty-rate describing the dirty page rate of vm >> +# in units of MB/s, present only when querying the >> +# rate has completed. > > How about: > > @dirty-rate: an estimate of the dirty page rate of the VM in units of > MB/s, present only when estimating the rate has completed. > That's better, thanks. >> # >> # @status: status containing dirtyrate query status includes >> # 'unstarted' or 'measuring' or 'measured' >> @@ -1759,7 +1758,7 @@ >> # >> ## >> { 'struct': 'DirtyRateInfo', >> - 'data': {'dirty-rate': 'int64', >> + 'data': {'*dirty-rate': 'int64', >> 'status': 'DirtyRateStatus', >> 'start-time': 'int64', >> 'calc-time': 'int64'} } >> -- >> 1.8.3.1 > > dme. >
diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c index 68577ef..9024b0f 100644 --- a/migration/dirtyrate.c +++ b/migration/dirtyrate.c @@ -69,9 +69,8 @@ static struct DirtyRateInfo *query_dirty_rate_info(void) struct DirtyRateInfo *info = g_malloc0(sizeof(DirtyRateInfo)); if (qatomic_read(&CalculatingState) == DIRTY_RATE_STATUS_MEASURED) { + info->has_dirty_rate = true; info->dirty_rate = dirty_rate; - } else { - info->dirty_rate = -1; } info->status = CalculatingState; diff --git a/qapi/migration.json b/qapi/migration.json index ce2216c..6e428f7 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -1743,10 +1743,9 @@ # # 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 returns '-1', it means querying has not -# yet started or completed. +# @dirty-rate: dirty-rate describing the dirty page rate of vm +# 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' @@ -1759,7 +1758,7 @@ # ## { 'struct': 'DirtyRateInfo', - 'data': {'dirty-rate': 'int64', + 'data': {'*dirty-rate': 'int64', 'status': 'DirtyRateStatus', 'start-time': 'int64', 'calc-time': 'int64'} }
Make dirty_rate field optional, present dirty rate only when querying the rate has completed. The qmp results is shown as follow: @unstarted: {"return":{"status":"unstarted","start-time":0,"calc-time":0},"id":"libvirt-12"} @measuring: {"return":{"status":"measuring","start-time":0,"calc-time":0},"id":"libvirt-14"} @measured: {"return":{"status":"measured","dirty-rate":4,"start-time":150146,"calc-time":1},"id":"libvirt-15"} Signed-off-by: Chuan Zheng <zhengchuan@huawei.com> --- migration/dirtyrate.c | 3 +-- qapi/migration.json | 9 ++++----- 2 files changed, 5 insertions(+), 7 deletions(-)