Re: [PATCH v5 5/8] multifd: Add zlib compression multifd support

From: Juan Quintela <quintela@redhat.com>
To: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
Cc: "Laurent Vivier" <lvivier@redhat.com>,
	"Thomas Huth" <thuth@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [PATCH v5 5/8] multifd: Add zlib compression multifd support
Date: Thu, 13 Feb 2020 21:24:59 +0100	[thread overview]
Message-ID: <87tv3ujkv8.fsf@secure.laptop> (raw)
In-Reply-To: <20200211184332.GH2751@work-vm> (David Alan Gilbert's message of "Tue, 11 Feb 2020 18:43:32 +0000")

"Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote:
> * Juan Quintela (quintela@redhat.com) wrote:
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  hw/core/qdev-properties.c    |   2 +-
>>  migration/Makefile.objs      |   1 +
>>  migration/migration.c        |   9 +
>>  migration/migration.h        |   1 +
>>  migration/multifd-zlib.c     | 325 +++++++++++++++++++++++++++++++++++
>>  migration/multifd.c          |   6 +
>>  migration/multifd.h          |   4 +
>>  qapi/migration.json          |   3 +-
>>  tests/qtest/migration-test.c |   6 +
>>  9 files changed, 355 insertions(+), 2 deletions(-)
>>  create mode 100644 migration/multifd-zlib.c
>
> Coupld of really minor points below to fix up, but other than those:
>
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
>>  }
>>  
>> +int migrate_multifd_zlib_level(void)
>> +{
>> +    MigrationState *s;
>> +
>> +    s = migrate_get_current();
>> +
>> +    return s->parameters.multifd_zlib_level;
>> +}
>> +
>
> Does this belong in the previous patch?

It is used only here.  Should not make any difference.

Anyways, changing it.

>
>>  int migrate_use_xbzrle(void)
>>  {
>>      MigrationState *s;
>> diff --git a/migration/migration.h b/migration/migration.h
>> index 3d23a0852e..95e9c196ff 100644
>> --- a/migration/migration.h
>> +++ b/migration/migration.h
>> @@ -301,6 +301,7 @@ bool migrate_use_multifd(void);
>>  bool migrate_pause_before_switchover(void);
>>  int migrate_multifd_channels(void);
>>  MultiFDMethod migrate_multifd_method(void);
>> +int migrate_multifd_zlib_level(void);
>> +        int flush = Z_NO_FLUSH;
>> +        unsigned long start = zs->total_out;
>> +
>> +        if (i == used  - 1) {
>
> Note an extra space there.
>

Fixed, thanks.