On Fri, Aug 23, 2019 at 05:27:01PM +0100, Dr. David Alan Gilbert wrote:
>* Wei Yang (richardw.y...@linux.intel.com) wrote:
>> Now postcopy is not compatible with compression. And we disable setting
>> these two capability at the same time. While we can still leverage
>> compress before postcopy is active, for example at the bulk stage.
>>
>> This patch skips compression when postcopy is active instead of
>> forbidding setting these capability at the same time.
>>
>> Signed-off-by: Wei Yang <richardw.y...@linux.intel.com>
>
>So I think this is OK, because ram_save_complete has a call to
>flush_compressed_data in it.
>
>However I think we should hold it until you figure out the other series
>that really enables compress; the problem is that as soon as you
>allow the capability, then there's nothing to distinguish this version
>and the version that fully enables it. So how would you stop this
>version being used as the source to the version which fully enables it?
>
>So I think if we do the other series, then you should start off like
>this and then remove the capability check right at the end.
>
Agree, it is not proper to leave this middle state.
>Dave
>
>> ---
>> migration/migration.c | 11 -----------
>> migration/ram.c | 10 ++++++++++
>> 2 files changed, 10 insertions(+), 11 deletions(-)
>>
>> diff --git a/migration/migration.c b/migration/migration.c
>> index 5a496addbd..33c373033d 100644
>> --- a/migration/migration.c
>> +++ b/migration/migration.c
>> @@ -995,17 +995,6 @@ static bool migrate_caps_check(bool *cap_list,
>> #endif
>>
>> if (cap_list[MIGRATION_CAPABILITY_POSTCOPY_RAM]) {
>> - if (cap_list[MIGRATION_CAPABILITY_COMPRESS]) {
>> - /* The decompression threads asynchronously write into RAM
>> - * rather than use the atomic copies needed to avoid
>> - * userfaulting. It should be possible to fix the decompression
>> - * threads for compatibility in future.
>> - */
>> - error_setg(errp, "Postcopy is not currently compatible "
>> - "with compression");
>> - return false;
>> - }
>> -
>> /* This check is reasonably expensive, so only when it's being
>> * set the first time, also it's only the destination that needs
>> * special support.
>> diff --git a/migration/ram.c b/migration/ram.c
>> index da12774a24..a0d3bc60b2 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -2384,6 +2384,16 @@ static bool save_page_use_compression(RAMState *rs)
>> return false;
>> }
>>
>> + /*
>> + * The decompression threads asynchronously write into RAM
>> + * rather than use the atomic copies needed to avoid
>> + * userfaulting. It should be possible to fix the decompression
>> + * threads for compatibility in future.
>> + */
>> + if (migration_in_postcopy()) {
>> + return false;
>> + }
>> +
>> /*
>> * If xbzrle is on, stop using the data compression after first
>> * round of migration even if compression is enabled. In theory,
>> --
>> 2.17.1
>>
>--
>Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
--
Wei Yang
Help you, Help me
