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(-)

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":{
-- 
2.30.2

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)

-- 
Peter Xu

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,

-- 
Peter Xu

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,

-- 
Peter Xu