| Message ID | 20170314102747.31395-1-fanc.fnst@cn.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Mar 14, 2017 at 06:27:47PM +0800, Chao Fan wrote: > In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. > It's also better for other tools to determine the cpu throttle > value in different architecture. > > Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > --- > hmp.c | 6 +++--- > migration/migration.c | 2 +- > qapi-schema.json | 8 ++++---- > 3 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/hmp.c b/hmp.c > index 261843f..7334bc5 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -215,9 +215,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) > info->ram->normal_bytes >> 10); > monitor_printf(mon, "dirty sync count: %" PRIu64 "\n", > info->ram->dirty_sync_count); > - if (info->ram->dirty_pages_rate) { > - monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n", > - info->ram->dirty_pages_rate); > + if (info->ram->dirty_bytes_rate) { > + monitor_printf(mon, "dirty bytes rate: %" PRIu64 " bytes/s\n", > + info->ram->dirty_bytes_rate); > } > if (info->ram->postcopy_requests) { > monitor_printf(mon, "postcopy request count: %" PRIu64 "\n", > diff --git a/migration/migration.c b/migration/migration.c > index 3dab684..7e7b62c 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -648,7 +648,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) > > if (s->state != MIGRATION_STATUS_COMPLETED) { > info->ram->remaining = ram_bytes_remaining(); > - info->ram->dirty_pages_rate = s->dirty_pages_rate; > + info->ram->dirty_bytes_rate = s->dirty_bytes_rate; > } > } > > diff --git a/qapi-schema.json b/qapi-schema.json > index 32b4a4b..83a45f8 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -565,9 +565,6 @@ > # > # @normal-bytes: number of normal bytes sent (since 1.2) > # > -# @dirty-pages-rate: number of pages dirtied by second by the > -# guest (since 1.3) > -# > # @mbps: throughput in megabits/sec. (since 1.6) > # > # @dirty-sync-count: number of times that dirty ram was synchronized (since 2.1) > @@ -575,12 +572,15 @@ > # @postcopy-requests: The number of page requests received from the destination > # (since 2.7) > # > +# @dirty-bytes-rate: how many bytes dirtied by second by the > +# guest (since 2.9) > +# > # Since: 0.14.0 > ## > { 'struct': 'MigrationStats', > 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , > 'duplicate': 'int', 'skipped': 'int', 'normal': 'int', > - 'normal-bytes': 'int', 'dirty-pages-rate' : 'int', > + 'normal-bytes': 'int', 'dirty-bytes-rate' : 'int', > 'mbps' : 'number', 'dirty-sync-count' : 'int', > 'postcopy-requests' : 'int' } } You can't do this - this breaks every single app that uses dirty-pages-rate. The only option is to add dirty-bytes-rate as a second field, and leave the existing field unchanged. Alternatively, output 'page-size' and let the application do the math. Regards, Daniel
On Tue, Mar 14, 2017 at 10:45:16AM +0000, Daniel P. Berrange wrote: >On Tue, Mar 14, 2017 at 06:27:47PM +0800, Chao Fan wrote: >> In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. >> It's also better for other tools to determine the cpu throttle >> value in different architecture. >> >> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> --- >> hmp.c | 6 +++--- >> migration/migration.c | 2 +- >> qapi-schema.json | 8 ++++---- >> 3 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/hmp.c b/hmp.c >> index 261843f..7334bc5 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -215,9 +215,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) >> info->ram->normal_bytes >> 10); >> monitor_printf(mon, "dirty sync count: %" PRIu64 "\n", >> info->ram->dirty_sync_count); >> - if (info->ram->dirty_pages_rate) { >> - monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n", >> - info->ram->dirty_pages_rate); >> + if (info->ram->dirty_bytes_rate) { >> + monitor_printf(mon, "dirty bytes rate: %" PRIu64 " bytes/s\n", >> + info->ram->dirty_bytes_rate); >> } >> if (info->ram->postcopy_requests) { >> monitor_printf(mon, "postcopy request count: %" PRIu64 "\n", >> diff --git a/migration/migration.c b/migration/migration.c >> index 3dab684..7e7b62c 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -648,7 +648,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) >> >> if (s->state != MIGRATION_STATUS_COMPLETED) { >> info->ram->remaining = ram_bytes_remaining(); >> - info->ram->dirty_pages_rate = s->dirty_pages_rate; >> + info->ram->dirty_bytes_rate = s->dirty_bytes_rate; >> } >> } >> >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 32b4a4b..83a45f8 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -565,9 +565,6 @@ >> # >> # @normal-bytes: number of normal bytes sent (since 1.2) >> # >> -# @dirty-pages-rate: number of pages dirtied by second by the >> -# guest (since 1.3) >> -# >> # @mbps: throughput in megabits/sec. (since 1.6) >> # >> # @dirty-sync-count: number of times that dirty ram was synchronized (since 2.1) >> @@ -575,12 +572,15 @@ >> # @postcopy-requests: The number of page requests received from the destination >> # (since 2.7) >> # >> +# @dirty-bytes-rate: how many bytes dirtied by second by the >> +# guest (since 2.9) >> +# >> # Since: 0.14.0 >> ## >> { 'struct': 'MigrationStats', >> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , >> 'duplicate': 'int', 'skipped': 'int', 'normal': 'int', >> - 'normal-bytes': 'int', 'dirty-pages-rate' : 'int', >> + 'normal-bytes': 'int', 'dirty-bytes-rate' : 'int', >> 'mbps' : 'number', 'dirty-sync-count' : 'int', >> 'postcopy-requests' : 'int' } } > >You can't do this - this breaks every single app that uses dirty-pages-rate. > >The only option is to add dirty-bytes-rate as a second field, and leave >the existing field unchanged. Alternatively, output 'page-size' and let >the application do the math. Hi Daniel. Thank you for you reply. I think adding dirty-bytes-rate is better. I will change and resend the new patch. Thanks, Chao Fan > >Regards, >Daniel >-- >|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| >|: http://libvirt.org -o- http://virt-manager.org :| >|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| > >
Chao Fan <fanc.fnst@cn.fujitsu.com> wrote: > In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. > It's also better for other tools to determine the cpu throttle > value in different architecture. > > Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> I agree with Daniel here, you can't change the meaning of a field. Look at skipped pages. It is zero know because it is not used anymore, but we can't drop it. I think it is better to expose page_size. We have now trasferred: bytes total: bytes duplicate: number of zero pages skipped: always zero. normal: number of normal pages normal_bytes: the same in bytes mbps: megabytes per second? I can't even remember this one dirty_sync_count: number of times we have go through the whole memory postcopy_requests = number of pages asked by postcopy faults? dirty_pages_rate = pages by some kind of unit And we haven't yet started with compression or xbzrle. I think that the best approach at this point is putting everything in pages except the things that don't make sense. We can put everything on bytes, but then everything is HUGE. Anyways, what do libvirt/management apps preffer? Later, Juan. > --- > hmp.c | 6 +++--- > migration/migration.c | 2 +- > qapi-schema.json | 8 ++++---- > 3 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/hmp.c b/hmp.c > index 261843f..7334bc5 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -215,9 +215,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) > info->ram->normal_bytes >> 10); > monitor_printf(mon, "dirty sync count: %" PRIu64 "\n", > info->ram->dirty_sync_count); > - if (info->ram->dirty_pages_rate) { > - monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n", > - info->ram->dirty_pages_rate); > + if (info->ram->dirty_bytes_rate) { > + monitor_printf(mon, "dirty bytes rate: %" PRIu64 " bytes/s\n", > + info->ram->dirty_bytes_rate); > } > if (info->ram->postcopy_requests) { > monitor_printf(mon, "postcopy request count: %" PRIu64 "\n", > diff --git a/migration/migration.c b/migration/migration.c > index 3dab684..7e7b62c 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -648,7 +648,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) > > if (s->state != MIGRATION_STATUS_COMPLETED) { > info->ram->remaining = ram_bytes_remaining(); > - info->ram->dirty_pages_rate = s->dirty_pages_rate; > + info->ram->dirty_bytes_rate = s->dirty_bytes_rate; > } > } > > diff --git a/qapi-schema.json b/qapi-schema.json > index 32b4a4b..83a45f8 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -565,9 +565,6 @@ > # > # @normal-bytes: number of normal bytes sent (since 1.2) > # > -# @dirty-pages-rate: number of pages dirtied by second by the > -# guest (since 1.3) > -# > # @mbps: throughput in megabits/sec. (since 1.6) > # > # @dirty-sync-count: number of times that dirty ram was synchronized (since 2.1) > @@ -575,12 +572,15 @@ > # @postcopy-requests: The number of page requests received from the destination > # (since 2.7) > # > +# @dirty-bytes-rate: how many bytes dirtied by second by the > +# guest (since 2.9) > +# > # Since: 0.14.0 > ## > { 'struct': 'MigrationStats', > 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , > 'duplicate': 'int', 'skipped': 'int', 'normal': 'int', > - 'normal-bytes': 'int', 'dirty-pages-rate' : 'int', > + 'normal-bytes': 'int', 'dirty-bytes-rate' : 'int', > 'mbps' : 'number', 'dirty-sync-count' : 'int', > 'postcopy-requests' : 'int' } }
On Tue, Mar 14, 2017 at 01:29:43PM +0100, Juan Quintela wrote: > Chao Fan <fanc.fnst@cn.fujitsu.com> wrote: > > In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. > > It's also better for other tools to determine the cpu throttle > > value in different architecture. > > > > Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> > > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > > I agree with Daniel here, you can't change the meaning of a field. Look > at skipped pages. It is zero know because it is not used anymore, but > we can't drop it. > > I think it is better to expose page_size. We have now > > trasferred: bytes > total: bytes > duplicate: number of zero pages > skipped: always zero. > normal: number of normal pages > normal_bytes: the same in bytes > mbps: megabytes per second? I can't even remember this one > dirty_sync_count: number of times we have go through the whole memory > postcopy_requests = number of pages asked by postcopy faults? > dirty_pages_rate = pages by some kind of unit > > And we haven't yet started with compression or xbzrle. I think that the > best approach at this point is putting everything in pages except the > things that don't make sense. > > We can put everything on bytes, but then everything is HUGE. > > Anyways, what do libvirt/management apps preffer? Since we have many fields already which are reported as page counts, I think just adding page size would be preferrable to having twice as many fields reported duplicating bytes + pages. The only reason to favour duplicating all fields to report bytes, is if we needed to vary page size to deal with huge pages (eg if some reported pages were 4kb and other reported pages with 2MB). You can easily just scale huge pages counts to be "normal" pages for purpose of reporting though. Regards, Daniel
On Tue, Mar 14, 2017 at 12:37:30PM +0000, Daniel P. Berrange wrote: >On Tue, Mar 14, 2017 at 01:29:43PM +0100, Juan Quintela wrote: >> Chao Fan <fanc.fnst@cn.fujitsu.com> wrote: >> > In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. >> > It's also better for other tools to determine the cpu throttle >> > value in different architecture. >> > >> > Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> >> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> >> I agree with Daniel here, you can't change the meaning of a field. Look >> at skipped pages. It is zero know because it is not used anymore, but >> we can't drop it. >> >> I think it is better to expose page_size. We have now >> >> trasferred: bytes >> total: bytes >> duplicate: number of zero pages >> skipped: always zero. >> normal: number of normal pages >> normal_bytes: the same in bytes >> mbps: megabytes per second? I can't even remember this one >> dirty_sync_count: number of times we have go through the whole memory >> postcopy_requests = number of pages asked by postcopy faults? >> dirty_pages_rate = pages by some kind of unit >> >> And we haven't yet started with compression or xbzrle. I think that the >> best approach at this point is putting everything in pages except the >> things that don't make sense. >> >> We can put everything on bytes, but then everything is HUGE. >> >> Anyways, what do libvirt/management apps preffer? > >Since we have many fields already which are reported as page counts, I >think just adding page size would be preferrable to having twice as many >fields reported duplicating bytes + pages. OK. > >The only reason to favour duplicating all fields to report bytes, is if >we needed to vary page size to deal with huge pages (eg if some reported >pages were 4kb and other reported pages with 2MB). You can easily just >scale huge pages counts to be "normal" pages for purpose of reporting Thank you very much for your help! Thanks, Chao Fan >though. >Regards, >Daniel >-- >|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| >|: http://libvirt.org -o- http://virt-manager.org :| >|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| > >
On Tue, Mar 14, 2017 at 01:29:43PM +0100, Juan Quintela wrote: >Chao Fan <fanc.fnst@cn.fujitsu.com> wrote: >> In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. >> It's also better for other tools to determine the cpu throttle >> value in different architecture. >> >> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > >I agree with Daniel here, you can't change the meaning of a field. Look >at skipped pages. It is zero know because it is not used anymore, but >we can't drop it. OK, I see. Thank you very much. > >I think it is better to expose page_size. We have now > >trasferred: bytes >total: bytes >duplicate: number of zero pages >skipped: always zero. >normal: number of normal pages >normal_bytes: the same in bytes >mbps: megabytes per second? I can't even remember this one >dirty_sync_count: number of times we have go through the whole memory >postcopy_requests = number of pages asked by postcopy faults? >dirty_pages_rate = pages by some kind of unit > >And we haven't yet started with compression or xbzrle. I think that the >best approach at this point is putting everything in pages except the >things that don't make sense. > >We can put everything on bytes, but then everything is HUGE. Yes, it is. > >Anyways, what do libvirt/management apps preffer? I think maybe both are OK. About this issue, I will change my method to get the bytes. Many thanks for your reply. Thanks, Chao Fan > >Later, Juan. > > >> --- >> hmp.c | 6 +++--- >> migration/migration.c | 2 +- >> qapi-schema.json | 8 ++++---- >> 3 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/hmp.c b/hmp.c >> index 261843f..7334bc5 100644 >> --- a/hmp.c >> +++ b/hmp.c >> @@ -215,9 +215,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) >> info->ram->normal_bytes >> 10); >> monitor_printf(mon, "dirty sync count: %" PRIu64 "\n", >> info->ram->dirty_sync_count); >> - if (info->ram->dirty_pages_rate) { >> - monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n", >> - info->ram->dirty_pages_rate); >> + if (info->ram->dirty_bytes_rate) { >> + monitor_printf(mon, "dirty bytes rate: %" PRIu64 " bytes/s\n", >> + info->ram->dirty_bytes_rate); >> } >> if (info->ram->postcopy_requests) { >> monitor_printf(mon, "postcopy request count: %" PRIu64 "\n", >> diff --git a/migration/migration.c b/migration/migration.c >> index 3dab684..7e7b62c 100644 >> --- a/migration/migration.c >> +++ b/migration/migration.c >> @@ -648,7 +648,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) >> >> if (s->state != MIGRATION_STATUS_COMPLETED) { >> info->ram->remaining = ram_bytes_remaining(); >> - info->ram->dirty_pages_rate = s->dirty_pages_rate; >> + info->ram->dirty_bytes_rate = s->dirty_bytes_rate; >> } >> } >> >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 32b4a4b..83a45f8 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -565,9 +565,6 @@ >> # >> # @normal-bytes: number of normal bytes sent (since 1.2) >> # >> -# @dirty-pages-rate: number of pages dirtied by second by the >> -# guest (since 1.3) >> -# >> # @mbps: throughput in megabits/sec. (since 1.6) >> # >> # @dirty-sync-count: number of times that dirty ram was synchronized (since 2.1) >> @@ -575,12 +572,15 @@ >> # @postcopy-requests: The number of page requests received from the destination >> # (since 2.7) >> # >> +# @dirty-bytes-rate: how many bytes dirtied by second by the >> +# guest (since 2.9) >> +# >> # Since: 0.14.0 >> ## >> { 'struct': 'MigrationStats', >> 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , >> 'duplicate': 'int', 'skipped': 'int', 'normal': 'int', >> - 'normal-bytes': 'int', 'dirty-pages-rate' : 'int', >> + 'normal-bytes': 'int', 'dirty-bytes-rate' : 'int', >> 'mbps' : 'number', 'dirty-sync-count' : 'int', >> 'postcopy-requests' : 'int' } } > >
On 03/14/2017 05:27 AM, Chao Fan wrote: > In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. > It's also better for other tools to determine the cpu throttle > value in different architecture. > > Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > --- In addition to the (good) comments you've gotten on not breaking existing fields, and the choice between a single new field giving the page size (with all fields favoring pages) or lots of new fields giving bytes, I have another comment: > @@ -575,12 +572,15 @@ > # @postcopy-requests: The number of page requests received from the destination > # (since 2.7) > # > +# @dirty-bytes-rate: how many bytes dirtied by second by the > +# guest (since 2.9) You've missed soft freeze. Is this really bug-fix quality to be adding it into the release this late in the game for 2.9, or should it be deferred to 2.10? And while this is just a new field to an existing command, rather than a new command entirely, it's also worth thinking about Markus' edict for testsuite coverage: https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00296.html
On Tue, Mar 14, 2017 at 09:54:34AM -0500, Eric Blake wrote: >On 03/14/2017 05:27 AM, Chao Fan wrote: >> In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. >> It's also better for other tools to determine the cpu throttle >> value in different architecture. >> >> Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> >> Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> --- > >In addition to the (good) comments you've gotten on not breaking >existing fields, and the choice between a single new field giving the >page size (with all fields favoring pages) or lots of new fields giving >bytes, I have another comment: > >> @@ -575,12 +572,15 @@ >> # @postcopy-requests: The number of page requests received from the destination >> # (since 2.7) >> # >> +# @dirty-bytes-rate: how many bytes dirtied by second by the >> +# guest (since 2.9) > >You've missed soft freeze. Is this really bug-fix quality to be adding >it into the release this late in the game for 2.9, or should it be >deferred to 2.10? Ok, I will change it. Many thanks for your help. Thanks, Chao Fan > >And while this is just a new field to an existing command, rather than a >new command entirely, it's also worth thinking about Markus' edict for >testsuite coverage: >https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg00296.html > >-- >Eric Blake eblake redhat com +1-919-301-3266 >Libvirt virtualization library http://libvirt.org >
On Tue, Mar 14, 2017 at 12:37:30PM +0000, Daniel P. Berrange wrote: >On Tue, Mar 14, 2017 at 01:29:43PM +0100, Juan Quintela wrote: >> Chao Fan <fanc.fnst@cn.fujitsu.com> wrote: >> > In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. >> > It's also better for other tools to determine the cpu throttle >> > value in different architecture. >> > >> > Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> >> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> >> >> I agree with Daniel here, you can't change the meaning of a field. Look >> at skipped pages. It is zero know because it is not used anymore, but >> we can't drop it. >> >> I think it is better to expose page_size. We have now >> >> trasferred: bytes >> total: bytes >> duplicate: number of zero pages >> skipped: always zero. >> normal: number of normal pages >> normal_bytes: the same in bytes >> mbps: megabytes per second? I can't even remember this one >> dirty_sync_count: number of times we have go through the whole memory >> postcopy_requests = number of pages asked by postcopy faults? >> dirty_pages_rate = pages by some kind of unit >> >> And we haven't yet started with compression or xbzrle. I think that the >> best approach at this point is putting everything in pages except the >> things that don't make sense. >> >> We can put everything on bytes, but then everything is HUGE. >> >> Anyways, what do libvirt/management apps preffer? > >Since we have many fields already which are reported as page counts, I >think just adding page size would be preferrable to having twice as many >fields reported duplicating bytes + pages. > >The only reason to favour duplicating all fields to report bytes, is if >we needed to vary page size to deal with huge pages (eg if some reported >pages were 4kb and other reported pages with 2MB). You can easily just >scale huge pages counts to be "normal" pages for purpose of reporting >though. >Regards, >Daniel Hi Juan and Daniel, I am wondering if it's OK to expose page_size in qmp or hmp, just like a new command 'info page_size'. If confirmed，I will make the new patch. Thanks, Chao Fan >-- >|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| >|: http://libvirt.org -o- http://virt-manager.org :| >|: http://entangle-photo.org -o- http://search.cpan.org/~danberr/ :| > >
On Wed, Mar 15, 2017 at 12:13:58PM +0800, Chao Fan wrote: > On Tue, Mar 14, 2017 at 12:37:30PM +0000, Daniel P. Berrange wrote: > >On Tue, Mar 14, 2017 at 01:29:43PM +0100, Juan Quintela wrote: > >> Chao Fan <fanc.fnst@cn.fujitsu.com> wrote: > >> > In hmp, dirty-bytes-rate is more friendly than dirty-pages-rate. > >> > It's also better for other tools to determine the cpu throttle > >> > value in different architecture. > >> > > >> > Signed-off-by: Chao Fan <fanc.fnst@cn.fujitsu.com> > >> > Signed-off-by: Li Zhijian <lizhijian@cn.fujitsu.com> > >> > >> I agree with Daniel here, you can't change the meaning of a field. Look > >> at skipped pages. It is zero know because it is not used anymore, but > >> we can't drop it. > >> > >> I think it is better to expose page_size. We have now > >> > >> trasferred: bytes > >> total: bytes > >> duplicate: number of zero pages > >> skipped: always zero. > >> normal: number of normal pages > >> normal_bytes: the same in bytes > >> mbps: megabytes per second? I can't even remember this one > >> dirty_sync_count: number of times we have go through the whole memory > >> postcopy_requests = number of pages asked by postcopy faults? > >> dirty_pages_rate = pages by some kind of unit > >> > >> And we haven't yet started with compression or xbzrle. I think that the > >> best approach at this point is putting everything in pages except the > >> things that don't make sense. > >> > >> We can put everything on bytes, but then everything is HUGE. > >> > >> Anyways, what do libvirt/management apps preffer? > > > >Since we have many fields already which are reported as page counts, I > >think just adding page size would be preferrable to having twice as many > >fields reported duplicating bytes + pages. > > > >The only reason to favour duplicating all fields to report bytes, is if > >we needed to vary page size to deal with huge pages (eg if some reported > >pages were 4kb and other reported pages with 2MB). You can easily just > >scale huge pages counts to be "normal" pages for purpose of reporting > >though. > > I am wondering if it's OK to expose page_size in qmp or hmp, just like > a new command 'info page_size'. > If confirmed，I will make the new patch. I'd suggest having it as a field of "info migrate" Regards, Daniel
diff --git a/hmp.c b/hmp.c index 261843f..7334bc5 100644 --- a/hmp.c +++ b/hmp.c @@ -215,9 +215,9 @@ void hmp_info_migrate(Monitor *mon, const QDict *qdict) info->ram->normal_bytes >> 10); monitor_printf(mon, "dirty sync count: %" PRIu64 "\n", info->ram->dirty_sync_count); - if (info->ram->dirty_pages_rate) { - monitor_printf(mon, "dirty pages rate: %" PRIu64 " pages\n", - info->ram->dirty_pages_rate); + if (info->ram->dirty_bytes_rate) { + monitor_printf(mon, "dirty bytes rate: %" PRIu64 " bytes/s\n", + info->ram->dirty_bytes_rate); } if (info->ram->postcopy_requests) { monitor_printf(mon, "postcopy request count: %" PRIu64 "\n", diff --git a/migration/migration.c b/migration/migration.c index 3dab684..7e7b62c 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -648,7 +648,7 @@ static void populate_ram_info(MigrationInfo *info, MigrationState *s) if (s->state != MIGRATION_STATUS_COMPLETED) { info->ram->remaining = ram_bytes_remaining(); - info->ram->dirty_pages_rate = s->dirty_pages_rate; + info->ram->dirty_bytes_rate = s->dirty_bytes_rate; } } diff --git a/qapi-schema.json b/qapi-schema.json index 32b4a4b..83a45f8 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -565,9 +565,6 @@ # # @normal-bytes: number of normal bytes sent (since 1.2) # -# @dirty-pages-rate: number of pages dirtied by second by the -# guest (since 1.3) -# # @mbps: throughput in megabits/sec. (since 1.6) # # @dirty-sync-count: number of times that dirty ram was synchronized (since 2.1) @@ -575,12 +572,15 @@ # @postcopy-requests: The number of page requests received from the destination # (since 2.7) # +# @dirty-bytes-rate: how many bytes dirtied by second by the +# guest (since 2.9) +# # Since: 0.14.0 ## { 'struct': 'MigrationStats', 'data': {'transferred': 'int', 'remaining': 'int', 'total': 'int' , 'duplicate': 'int', 'skipped': 'int', 'normal': 'int', - 'normal-bytes': 'int', 'dirty-pages-rate' : 'int', + 'normal-bytes': 'int', 'dirty-bytes-rate' : 'int', 'mbps' : 'number', 'dirty-sync-count' : 'int', 'postcopy-requests' : 'int' } }