Re: [PATCH v2 3/7] migration/ram: Move RAM_SAVE_FLAG* into ram.h

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> Firstly, we're going to use the multifd flag soon in multifd code, so ram.c
> isn't gonna work.
>
> Secondly, we have a separate RDMA flag dangling around, which is definitely
> not obvious.  There's one comment that helps, but not too much.
>
> We should just put it altogether, so nothing will get overlooked.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

just some comments about preexisting stuff:

> ---
>  migration/ram.h  | 25 +++++++++++++++++++++++++
>  migration/rdma.h |  7 -------
>  migration/ram.c  | 21 ---------------------
>  3 files changed, 25 insertions(+), 28 deletions(-)
>
> diff --git a/migration/ram.h b/migration/ram.h
> index 0d1981f888..cfdcccd266 100644
> --- a/migration/ram.h
> +++ b/migration/ram.h
> @@ -33,6 +33,31 @@
>  #include "exec/cpu-common.h"
>  #include "io/channel.h"
>  
> +/*
> + * RAM_SAVE_FLAG_ZERO used to be named RAM_SAVE_FLAG_COMPRESS, it
> + * worked for pages that were filled with the same char.  We switched
> + * it to only search for the zero value.  And to avoid confusion with
> + * RAM_SAVE_FLAG_COMPRESS_PAGE just rename it.
> + *
> + * RAM_SAVE_FLAG_FULL was obsoleted in 2009.
> + *
> + * RAM_SAVE_FLAG_COMPRESS_PAGE (0x100) was removed in QEMU 9.1.

Aren't these already covered by git log? The comment makes it seem like
some special situation, but I think we're just documenting history here,
no?

> + */
> +#define RAM_SAVE_FLAG_FULL     0x01
> +#define RAM_SAVE_FLAG_ZERO     0x02
> +#define RAM_SAVE_FLAG_MEM_SIZE 0x04
> +#define RAM_SAVE_FLAG_PAGE     0x08
> +#define RAM_SAVE_FLAG_EOS      0x10
> +#define RAM_SAVE_FLAG_CONTINUE 0x20
> +#define RAM_SAVE_FLAG_XBZRLE   0x40
> +/*
> + * ONLY USED IN RDMA: Whenever this is found in the data stream, the flags
> + * will be passed to rdma functions in the incoming-migration side.
> + */
> +#define RAM_SAVE_FLAG_HOOK     0x80

No 0x100?

> +#define RAM_SAVE_FLAG_MULTIFD_FLUSH    0x200
> +/* We can't use any flag that is bigger than 0x200 */

Where does that limitation come from again? I know that
RAM_SAVE_FLAG_MEM_SIZE shares a u64 with something else:

  qemu_put_be64(f, ram_bytes_total_with_ignored() |
  RAM_SAVE_FLAG_MEM_SIZE);

For RAM_SAVE_FLAG_ZERO and RAM_SAVE_FLAG_PAGE, it might be a u32 (offset
is ram_addr_t):

  save_page_header(pss, file, pss->block, offset | RAM_SAVE_FLAG_ZERO);

But others just go by themselves:

qemu_put_be64(f, RAM_SAVE_FLAG_MULTIFD_FLUSH);


> +
>  extern XBZRLECacheStats xbzrle_counters;
>  
>  /* Should be holding either ram_list.mutex, or the RCU lock. */
> diff --git a/migration/rdma.h b/migration/rdma.h
> index a8d27f33b8..f55f28bbed 100644
> --- a/migration/rdma.h
> +++ b/migration/rdma.h
> @@ -33,13 +33,6 @@ void rdma_start_incoming_migration(InetSocketAddress 
> *host_port, Error **errp);
>  #define RAM_CONTROL_ROUND     1
>  #define RAM_CONTROL_FINISH    3
>  
> -/*
> - * Whenever this is found in the data stream, the flags
> - * will be passed to rdma functions in the incoming-migration
> - * side.
> - */
> -#define RAM_SAVE_FLAG_HOOK     0x80
> -
>  #define RAM_SAVE_CONTROL_NOT_SUPP -1000
>  #define RAM_SAVE_CONTROL_DELAYED  -2000
>  
> diff --git a/migration/ram.c b/migration/ram.c
> index 7284c34bd8..44010ff325 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -71,27 +71,6 @@
>  /***********************************************************/
>  /* ram save/restore */
>  
> -/*
> - * RAM_SAVE_FLAG_ZERO used to be named RAM_SAVE_FLAG_COMPRESS, it
> - * worked for pages that were filled with the same char.  We switched
> - * it to only search for the zero value.  And to avoid confusion with
> - * RAM_SAVE_FLAG_COMPRESS_PAGE just rename it.
> - *
> - * RAM_SAVE_FLAG_FULL was obsoleted in 2009.
> - *
> - * RAM_SAVE_FLAG_COMPRESS_PAGE (0x100) was removed in QEMU 9.1.
> - */
> -#define RAM_SAVE_FLAG_FULL     0x01
> -#define RAM_SAVE_FLAG_ZERO     0x02
> -#define RAM_SAVE_FLAG_MEM_SIZE 0x04
> -#define RAM_SAVE_FLAG_PAGE     0x08
> -#define RAM_SAVE_FLAG_EOS      0x10
> -#define RAM_SAVE_FLAG_CONTINUE 0x20
> -#define RAM_SAVE_FLAG_XBZRLE   0x40
> -/* 0x80 is reserved in rdma.h for RAM_SAVE_FLAG_HOOK */
> -#define RAM_SAVE_FLAG_MULTIFD_FLUSH    0x200
> -/* We can't use any flag that is bigger than 0x200 */
> -
>  /*
>   * mapped-ram migration supports O_DIRECT, so we need to make sure the
>   * userspace buffer, the IO operation size and the file offset are