Peter Xu <pet...@redhat.com> wrote:
> On Tue, Jun 13, 2017 at 11:54:31AM +0200, Juan Quintela wrote:
>> Right now, if we receive a compressed page while this features are
>> disabled, Bad Things (TM) can happen. Just add a test for them.
>>
>> Signed-off-by: Juan Quintela <quint...@redhat.com>
>> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
>>
>> --
>>
>> I had XBZRLE here also, but it don't need extra resources on
>> destination, only on source. Additionally libvirt don't enable it on
>> destination, so don't put it here.
>> ---
>> migration/ram.c | 16 +++++++++++++++-
>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/migration/ram.c b/migration/ram.c
>> index f35d65a..f2d1bce 100644
>> --- a/migration/ram.c
>> +++ b/migration/ram.c
>> @@ -2477,7 +2477,7 @@ static int ram_load_postcopy(QEMUFile *f)
>>
>> static int ram_load(QEMUFile *f, void *opaque, int version_id)
>> {
>> - int flags = 0, ret = 0;
>> + int flags = 0, ret = 0, invalid_flags;
>> static uint64_t seq_iter;
>> int len = 0;
>> /*
>> @@ -2494,6 +2494,11 @@ static int ram_load(QEMUFile *f, void *opaque, int
>> version_id)
>> ret = -EINVAL;
>> }
>>
>> + invalid_flags = 0;
>
> Nit: set zero when declare (just like flags, ret)?
Fixed.
As answered to dave, I use to do it that way.
But here it is more consistant to do it the other way.
>> +
>> + if (!migrate_use_compression()) {
>> + invalid_flags |= RAM_SAVE_FLAG_COMPRESS_PAGE;
>> + }
>> /* This RCU critical section can be very long running.
>> * When RCU reclaims in the code start to become numerous,
>> * it will be necessary to reduce the granularity of this
>> @@ -2514,6 +2519,15 @@ static int ram_load(QEMUFile *f, void *opaque, int
>> version_id)
>> flags = addr & ~TARGET_PAGE_MASK;
>> addr &= TARGET_PAGE_MASK;
>>
>> + if (flags & invalid_flags) {
>> + if (flags & invalid_flags & RAM_SAVE_FLAG_COMPRESS_PAGE) {
> ^
> Nit: one extra space -------------------+
Fixed.
>> + error_report("Received an unexpected compressed page");
>> + }
>> +
>> + ret = -EINVAL;
>> + break;
>> + }
>> +
>> if (flags & (RAM_SAVE_FLAG_ZERO | RAM_SAVE_FLAG_PAGE |
>> RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
>> RAMBlock *block = ram_block_from_stream(f, flags);
>> --
>> 2.9.4
>>
>
> Besides the nits:
>
> Reviewed-by: Peter Xu <pet...@redhat.com>
Thanks.
