Re: [Qemu-devel] [PATCH 1/2] migration: Test for disabled features on reception

[Peter Xu]

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 <address@hidden>
> Reviewed-by: Dr. David Alan Gilbert <address@hidden>
> 
> --
> 
> 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)?

> +
> +    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 -------------------+

> +                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 <address@hidden>

-- 
Peter Xu