Re: [PATCH v2 8/9] tools/xl: mark LZ4 migration streams with a mandatory header flag

[Teddy Astie <teddy.astie@xxxxxxxxxx>]

Le 20/10/2026 à 22:06, Marcus Granado a écrit :
> A receiver which cannot decompress LZ4 rejects the first compressed
> PAGE_DATA record.  That is late: the receiver has already created the
> domain, precopy has started, and an older libxenguest's error says
> nothing about compression.
>
> xl can fail earlier.  Its save file header carries mandatory_flags, and
> create_domain() rejects unknown bits before it calls
> libxl_domain_create_restore().  Similar to commit e72ce244dc91
> ("tools/xl: Mandatory flag indicating the format of the migration
> stream"), claim bit 2 for LZ4 and set it for "xl migrate --compress".
> An older xl then reports mandatory flag 0x4 as unsupported and creates
> no domain.
>
> This is defence in depth, for xl-to-xl migration only.  An xl which
> knows the flag but whose libxenguest lacks liblz4 still accepts the
> header and fails late, at the first compressed PAGE_DATA record, though
> with an error that names LZ4.  The header is written before any record,
> so a stream in which no record ends up compressed still carries the
> flag and needs an LZ4-aware xl.
>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Marcus Granado <marcus.granado@xxxxxxxxxx>
> ---
I think it would be preferable to check for libxenguest lz4 support as 
well, so that libxenguest without lz4 would fail the same way as 
libxenguest prior to this patch.
> Notes:
>      Changes in v2:
>      - New, after an off-list question from Ross Lagerwall: an older xl
>        now refuses a compressed stream before it creates the domain.
>
>   tools/xl/xl.h             | 7 +++++--
>   tools/xl/xl_migrate.c     | 8 ++++----
>   tools/xl/xl_saverestore.c | 7 +++++--
>   3 files changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/tools/xl/xl.h b/tools/xl/xl.h
> index 7b75f9a493..009cbd1c3c 100644
> --- a/tools/xl/xl.h
> +++ b/tools/xl/xl.h
> @@ -77,8 +77,10 @@ static const char migrate_report[]=
>   
>   #define XL_MANDATORY_FLAG_JSON (1U << 0) /* config data is in JSON format */
>   #define XL_MANDATORY_FLAG_STREAMv2 (1U << 1) /* stream is v2 */
> +#define XL_MANDATORY_FLAG_COMPRESS_LZ4 (1U << 2) /* PAGE_DATA may use LZ4 */
>   #define XL_MANDATORY_FLAG_ALL  (XL_MANDATORY_FLAG_JSON |        \
> -                                XL_MANDATORY_FLAG_STREAMv2)
> +                                XL_MANDATORY_FLAG_STREAMv2 |    \
> +                                XL_MANDATORY_FLAG_COMPRESS_LZ4)
>   
>   struct save_file_header {
>       char magic[32]; /* savefileheader_magic */
> @@ -102,7 +104,8 @@ void save_domain_core_begin(uint32_t domid,
>                               uint8_t **config_data_r,
>                               int *config_len_r);
>   void save_domain_core_writeconfig(int fd, const char *source,
> -                                  const uint8_t *config_data, int config_len);
> +                                  const uint8_t *config_data, int config_len,
> +                                  int compress_lz4);
>   
>   /*
>    * The xl process should always return either EXIT_SUCCESS or
> diff --git a/tools/xl/xl_migrate.c b/tools/xl/xl_migrate.c
> index 31a8cb4f32..230aac1b71 100644
> --- a/tools/xl/xl_migrate.c
> +++ b/tools/xl/xl_migrate.c
> @@ -153,7 +153,7 @@ static void migration_child_report(int recv_fd) {
>   
>   static void migrate_do_preamble(int send_fd, int recv_fd, pid_t child,
>                                   uint8_t *config_data, int config_len,
> -                                const char *rune)
> +                                const char *rune, int compress_lz4)
>   {
>       int rc = 0;
>   
> @@ -172,7 +172,7 @@ static void migrate_do_preamble(int send_fd, int recv_fd, pid_t child,
>       }
>   
>       save_domain_core_writeconfig(send_fd, "migration stream",
> -                                 config_data, config_len);
> +                                 config_data, config_len, compress_lz4);
>   
>   }
>   
> @@ -200,7 +200,7 @@ static void migrate_domain(uint32_t domid, int preserve_domid,
>       child = create_migration_child(rune, &send_fd, &recv_fd);
>   
>       migrate_do_preamble(send_fd, recv_fd, child, config_data, config_len,
> -                        rune);
> +                        rune, compress_lz4);
>   
>       xtl_stdiostream_adjust_flags(logger, XTL_STDIOSTREAM_HIDE_PROGRESS, 0);
>   
> @@ -740,7 +740,7 @@ int main_remus(int argc, char **argv)
>           child = create_migration_child(rune, &send_fd, &recv_fd);
>   
>           migrate_do_preamble(send_fd, recv_fd, child, config_data, config_len,
> -                            rune);
> +                            rune, 0);
>   
>           if (ssh_command[0])
>               free(rune);
> diff --git a/tools/xl/xl_saverestore.c b/tools/xl/xl_saverestore.c
> index 747094ec7b..5151c8a130 100644
> --- a/tools/xl/xl_saverestore.c
> +++ b/tools/xl/xl_saverestore.c
> @@ -79,7 +79,8 @@ void save_domain_core_begin(uint32_t domid,
>   }
>   
>   void save_domain_core_writeconfig(int fd, const char *source,
> -                                  const uint8_t *config_data, int config_len)
> +                                  const uint8_t *config_data, int config_len,
> +                                  int compress_lz4)
>   {
>       struct save_file_header hdr;
>       uint8_t *optdata_begin;
> @@ -89,6 +90,8 @@ void save_domain_core_writeconfig(int fd, const char *source,
>       memcpy(hdr.magic, savefileheader_magic, sizeof(hdr.magic));
>       hdr.byteorder = SAVEFILE_BYTEORDER_VALUE;
>       hdr.mandatory_flags = XL_MANDATORY_FLAG_STREAMv2;
> +    if (compress_lz4)
> +        hdr.mandatory_flags |= XL_MANDATORY_FLAG_COMPRESS_LZ4;
>   
>       optdata_begin= 0;
>   
> @@ -144,7 +147,7 @@ static int save_domain(uint32_t domid, int preserve_domid,
>           exit(EXIT_FAILURE);
>       }
>   
> -    save_domain_core_writeconfig(fd, filename, config_data, config_len);
> +    save_domain_core_writeconfig(fd, filename, config_data, config_len, 0);
>   
>       int rc = libxl_domain_suspend(ctx, domid, fd, 0, NULL);
>       close(fd);

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature