| Message ID | 20240206231908.1792529-3-hao.xiang@bytedance.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce multifd zero page checking. | expand |
On Tue, Feb 06, 2024 at 11:19:04PM +0000, Hao Xiang wrote: > This change extends the MigrationStatus interface to track zero pages > and zero bytes counter. > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> Reviewed-by: Peter Xu <peterx@redhat.com> When post anything QAPI relevant, please always remember to copy QAPI maintainers too, thanks. $ ./scripts/get_maintainer.pl -f qapi/migration.json Eric Blake <eblake@redhat.com> (supporter:QAPI Schema) Markus Armbruster <armbru@redhat.com> (supporter:QAPI Schema) Peter Xu <peterx@redhat.com> (maintainer:Migration) Fabiano Rosas <farosas@suse.de> (maintainer:Migration) qemu-devel@nongnu.org (open list:All patches CC here)
On Wed, Feb 07, 2024 at 12:13:10PM +0800, Peter Xu wrote: > On Tue, Feb 06, 2024 at 11:19:04PM +0000, Hao Xiang wrote: > > This change extends the MigrationStatus interface to track zero pages > > and zero bytes counter. > > > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> > > Reviewed-by: Peter Xu <peterx@redhat.com> I'll need to scratch this, sorry.. The issue is I forgot we have "duplicate" which is exactly "zero page"s.. See: info->ram->duplicate = stat64_get(&mig_stats.zero_pages); If you think the name too confusing and want a replacement, maybe it's fine and maybe we can do that. Then we can keep this zero page counter introduced, reporting the same value as duplicates, then with a follow up patch to deprecate "duplicate" parameter. See an exmaple on how to deprecate in 7b24d326348e1672. One thing I'm not sure is whether Libvirt will be fine on losing "duplicates" after 2+ QEMU major releases. Copy Jiri for this. My understanding is that Libvirt should be keeping an eye on deprecation list and react, but I'd like to double check.. Or we can keep using "duplicates", but I agree it just reads weird.. Thanks,
On Wed, Feb 07, 2024 at 12:37:15 +0800, Peter Xu wrote: > On Wed, Feb 07, 2024 at 12:13:10PM +0800, Peter Xu wrote: > > On Tue, Feb 06, 2024 at 11:19:04PM +0000, Hao Xiang wrote: > > > This change extends the MigrationStatus interface to track zero pages > > > and zero bytes counter. > > > > > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> > > > > Reviewed-by: Peter Xu <peterx@redhat.com> > > I'll need to scratch this, sorry.. > > The issue is I forgot we have "duplicate" which is exactly "zero > page"s.. See: > > info->ram->duplicate = stat64_get(&mig_stats.zero_pages); > > If you think the name too confusing and want a replacement, maybe it's fine > and maybe we can do that. Then we can keep this zero page counter > introduced, reporting the same value as duplicates, then with a follow up > patch to deprecate "duplicate" parameter. See an exmaple on how to > deprecate in 7b24d326348e1672. > > One thing I'm not sure is whether Libvirt will be fine on losing > "duplicates" after 2+ QEMU major releases. Copy Jiri for this. My > understanding is that Libvirt should be keeping an eye on deprecation list > and react, but I'd like to double check.. This should not be a big deal as we can internally map either one (depending on what QEMU supports) to the same libvirt's field. AFAIK there is a consensus on Cc-ing libvirt-devel on patches that deprecate QEMU interfaces so that we can update our code in time before the deprecated interface is dropped. BTW, libvirt maps "duplicate" to: /** * VIR_DOMAIN_JOB_MEMORY_CONSTANT: * * virDomainGetJobStats field: number of pages filled with a constant * byte (all bytes in a single page are identical) transferred since the * beginning of the migration job, as VIR_TYPED_PARAM_ULLONG. * * The most common example of such pages are zero pages, i.e., pages filled * with zero bytes. * * Since: 1.0.3 */ # define VIR_DOMAIN_JOB_MEMORY_CONSTANT "memory_constant" Jirka
On Wed, Feb 7, 2024 at 12:41 AM Jiri Denemark <jdenemar@redhat.com> wrote: > > On Wed, Feb 07, 2024 at 12:37:15 +0800, Peter Xu wrote: > > On Wed, Feb 07, 2024 at 12:13:10PM +0800, Peter Xu wrote: > > > On Tue, Feb 06, 2024 at 11:19:04PM +0000, Hao Xiang wrote: > > > > This change extends the MigrationStatus interface to track zero pages > > > > and zero bytes counter. > > > > > > > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> > > > > > > Reviewed-by: Peter Xu <peterx@redhat.com> > > > > I'll need to scratch this, sorry.. > > > > The issue is I forgot we have "duplicate" which is exactly "zero > > page"s.. See: > > > > info->ram->duplicate = stat64_get(&mig_stats.zero_pages); > > > > If you think the name too confusing and want a replacement, maybe it's fine > > and maybe we can do that. Then we can keep this zero page counter > > introduced, reporting the same value as duplicates, then with a follow up > > patch to deprecate "duplicate" parameter. See an exmaple on how to > > deprecate in 7b24d326348e1672. > > > > One thing I'm not sure is whether Libvirt will be fine on losing > > "duplicates" after 2+ QEMU major releases. Copy Jiri for this. My > > understanding is that Libvirt should be keeping an eye on deprecation list > > and react, but I'd like to double check.. > > This should not be a big deal as we can internally map either one > (depending on what QEMU supports) to the same libvirt's field. AFAIK > there is a consensus on Cc-ing libvirt-devel on patches that deprecate > QEMU interfaces so that we can update our code in time before the > deprecated interface is dropped. > > BTW, libvirt maps "duplicate" to: > > /** > * VIR_DOMAIN_JOB_MEMORY_CONSTANT: > * > * virDomainGetJobStats field: number of pages filled with a constant > * byte (all bytes in a single page are identical) transferred since the > * beginning of the migration job, as VIR_TYPED_PARAM_ULLONG. > * > * The most common example of such pages are zero pages, i.e., pages filled > * with zero bytes. > * > * Since: 1.0.3 > */ > # define VIR_DOMAIN_JOB_MEMORY_CONSTANT "memory_constant" > > Jirka > Interesting. I didn't notice the existence of "duplicate" for zero pages. I do think the name is quite confusing. I will create the "zero/zero_bytes" counter and a separate commit to deprecate "duplicate". Will add libvirt devs per instruction above.
On Wed, Feb 07, 2024 at 03:44:18PM -0800, Hao Xiang wrote: > On Wed, Feb 7, 2024 at 12:41 AM Jiri Denemark <jdenemar@redhat.com> wrote: > > > > On Wed, Feb 07, 2024 at 12:37:15 +0800, Peter Xu wrote: > > > On Wed, Feb 07, 2024 at 12:13:10PM +0800, Peter Xu wrote: > > > > On Tue, Feb 06, 2024 at 11:19:04PM +0000, Hao Xiang wrote: > > > > > This change extends the MigrationStatus interface to track zero pages > > > > > and zero bytes counter. > > > > > > > > > > Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> > > > > > > > > Reviewed-by: Peter Xu <peterx@redhat.com> > > > > > > I'll need to scratch this, sorry.. > > > > > > The issue is I forgot we have "duplicate" which is exactly "zero > > > page"s.. See: > > > > > > info->ram->duplicate = stat64_get(&mig_stats.zero_pages); > > > > > > If you think the name too confusing and want a replacement, maybe it's fine > > > and maybe we can do that. Then we can keep this zero page counter > > > introduced, reporting the same value as duplicates, then with a follow up > > > patch to deprecate "duplicate" parameter. See an exmaple on how to > > > deprecate in 7b24d326348e1672. > > > > > > One thing I'm not sure is whether Libvirt will be fine on losing > > > "duplicates" after 2+ QEMU major releases. Copy Jiri for this. My > > > understanding is that Libvirt should be keeping an eye on deprecation list > > > and react, but I'd like to double check.. > > > > This should not be a big deal as we can internally map either one > > (depending on what QEMU supports) to the same libvirt's field. AFAIK > > there is a consensus on Cc-ing libvirt-devel on patches that deprecate I see. > > QEMU interfaces so that we can update our code in time before the > > deprecated interface is dropped. Right. What I mostly worried is "old libvirt" + "new qemu", where the old libvirt only knows "duplicates", while the new (after 2 releases) will only report "zeros". > > > > BTW, libvirt maps "duplicate" to: > > > > /** > > * VIR_DOMAIN_JOB_MEMORY_CONSTANT: > > * > > * virDomainGetJobStats field: number of pages filled with a constant > > * byte (all bytes in a single page are identical) transferred since the > > * beginning of the migration job, as VIR_TYPED_PARAM_ULLONG. > > * > > * The most common example of such pages are zero pages, i.e., pages filled > > * with zero bytes. > > * > > * Since: 1.0.3 > > */ > > # define VIR_DOMAIN_JOB_MEMORY_CONSTANT "memory_constant" > > > > Jirka > > > > Interesting. I didn't notice the existence of "duplicate" for zero > pages. I do think the name is quite confusing. I will create the > "zero/zero_bytes" counter and a separate commit to deprecate > "duplicate". Will add libvirt devs per instruction above. Yeah, please go ahead, and I hope my worry is not a real concern above; we can figure that out later. Even without deprecating "duplicate", maybe it'll at least still be worthwhile we start having "zeros" reported alongside. Then after 10/20/30/N years we always have a chance to deprecate the other one, just a matter of compatible window. Thanks,
diff --git a/qapi/migration.json b/qapi/migration.json index ff033a0344..69366fe3f4 100644 --- a/qapi/migration.json +++ b/qapi/migration.json @@ -63,6 +63,10 @@ # between 0 and @dirty-sync-count * @multifd-channels. (since # 7.1) # +# @zero: number of zero pages (since 9.0) +# +# @zero-bytes: number of zero bytes sent (since 9.0) +# # Features: # # @deprecated: Member @skipped is always zero since 1.5.3 @@ -81,7 +85,8 @@ 'multifd-bytes': 'uint64', 'pages-per-second': 'uint64', 'precopy-bytes': 'uint64', 'downtime-bytes': 'uint64', 'postcopy-bytes': 'uint64', - 'dirty-sync-missed-zero-copy': 'uint64' } } + 'dirty-sync-missed-zero-copy': 'uint64', + 'zero': 'int', 'zero-bytes': 'int' } } ## # @XBZRLECacheStats: @@ -332,6 +337,8 @@ # "duplicate":123, # "normal":123, # "normal-bytes":123456, +# "zero":123, +# "zero-bytes":123456, # "dirty-sync-count":15 # } # } @@ -358,6 +365,8 @@ # "duplicate":123, # "normal":123, # "normal-bytes":123456, +# "zero":123, +# "zero-bytes":123456, # "dirty-sync-count":15 # } # } @@ -379,6 +388,8 @@ # "duplicate":123, # "normal":123, # "normal-bytes":123456, +# "zero":123, +# "zero-bytes":123456, # "dirty-sync-count":15 # }, # "disk":{ @@ -405,6 +416,8 @@ # "duplicate":10, # "normal":3333, # "normal-bytes":3412992, +# "zero":3333, +# "zero-bytes":3412992, # "dirty-sync-count":15 # }, # "xbzrle-cache":{
This change extends the MigrationStatus interface to track zero pages and zero bytes counter. Signed-off-by: Hao Xiang <hao.xiang@bytedance.com> --- qapi/migration.json | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)