Re: [RFC 1/6] migration: skipped field is really obsolete.

[Juan Quintela <quintela@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Mon, Jun 12, 2023 at 09:33:39PM +0200, Juan Quintela wrote:
>> Has return zero for more than 10 years.  Just mark it deprecated.
>
> Specifically we introduced the field in 1.5.0
>
> commit f1c72795af573b24a7da5eb52375c9aba8a37972
> Author: Peter Lieven <pl@kamp.de>
> Date:   Tue Mar 26 10:58:37 2013 +0100
>
>     migration: do not sent zero pages in bulk stage
>     
>     during bulk stage of ram migration if a page is a
>     zero page do not send it at all.
>     the memory at the destination reads as zero anyway.
>     
>     even if there is an madvise with QEMU_MADV_DONTNEED
>     at the target upon receipt of a zero page I have observed
>     that the target starts swapping if the memory is overcommitted.
>     it seems that the pages are dropped asynchronously.
>     
>     this patch also updates QMP to return the number of
>     skipped pages in MigrationStats.
>     
>
>
> but removed its usage in 1.5.3
>
> commit 9ef051e5536b6368a1076046ec6c4ec4ac12b5c6
> Author: Peter Lieven <pl@kamp.de>
> Date:   Mon Jun 10 12:14:19 2013 +0200
>
>     Revert "migration: do not sent zero pages in bulk stage"
>     
>     Not sending zero pages breaks migration if a page is zero
>     at the source but not at the destination. This can e.g. happen
>     if different BIOS versions are used at source and destination.
>     It has also been reported that migration on pseries is completely
>     broken with this patch.
>     
>     This effectively reverts commit f1c72795af573b24a7da5eb52375c9aba8a37972.


Thanks for the history O:-)

>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  docs/about/deprecated.rst | 10 ++++++++++
>>  qapi/migration.json       | 12 ++++++++++--
>>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index cb7cd3e578..bcae193733 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -23,7 +23,8 @@
>>  #
>>  # @duplicate: number of duplicate (zero) pages (since 1.2)
>>  #
>> -# @skipped: number of skipped zero pages (since 1.5)
>> +# @skipped: number of skipped zero pages. Don't use, only provided for
>> +#     compatibility (since 1.5)
>
> I'd say
>
>    @skipped: number of skipped zero pages. Always zero, only provided for
>    compatibility (since 1.5)

Changed.

>>  #
>>  # @normal: number of normal pages (since 1.2)
>>  #
>> @@ -62,11 +63,18 @@
>>  #     between 0 and @dirty-sync-count * @multifd-channels.  (since
>>  #     7.1)
>>  #
>> +# Features:
>> +#
>> +# @deprecated: Member @skipped has not been used for a long time.
>
>   @deprecated: Member @skipped is always zero since 1.5.3

Changed.

Thanks.