Re: [PATCH v2 7/9] tools/libxl,xl: add a --compress option to xl migrate

[Teddy Astie <teddy.astie@xxxxxxxxxx>]

Le 20/10/2026 à 22:05, Marcus Granado a écrit :
> libxenguest can compress PAGE_DATA records with LZ4 when its caller
> sets XCFLAGS_COMPRESS_LZ4, but libxl never sets it, so xl has no way to
> ask for compression.
>
> Add a LIBXL_SUSPEND_COMPRESS_LZ4 flag for libxl_domain_suspend(), pass
> it to libxenguest as XCFLAGS_COMPRESS_LZ4, and expose it as
> "xl migrate --compress".
>
> Off by default: whether compressing pays off depends on how compressible
> the guest's memory is and on the CPU headroom of both hosts, so the
> caller decides.  Only xl migrate gets the option; xl save and xl remus
> never use LZ4.
>
> The receiving side needs no option, as each PAGE_DATA record says
> whether it is compressed.  A receiver which cannot decompress rejects
> the first compressed record, after it has created the domain, and the
> migration fails with the guest still on the source.  A sender whose
> libxenguest was built without a supported liblz4 fails the migration
> before sending any guest memory; libxl reports it as a suspend timeout.
>
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Marcus Granado <marcus.granado@xxxxxxxxxx>
> ---
I wonder if we should name the option "--compress-lz4", like it is used 
in the code (compress_lz4, XCFLAGS_COMPRESS_LZ4, 
LIBXL_SUSPEND_COMPRESS_LZ4, ...).
> Notes:
>      Changes in v2:
>      - Was v1 6/6.  The libxl and xl code is unchanged; the help text and
>        man page now say both hosts need liblz4 support in libxenguest, and
>        CHANGELOG.md has an entry.
>      
>      No LIBXL_HAVE_* macro: LIBXL_SUSPEND_COMPRESS_LZ4 is itself a #define,
>      so a caller can test for it with #ifdef LIBXL_SUSPEND_COMPRESS_LZ4.
>
>   CHANGELOG.md                      |  3 +++
>   docs/man/xl.1.pod.in              |  8 ++++++++
>   tools/include/libxl.h             |  1 +
>   tools/libs/light/libxl_dom_save.c |  4 +++-
>   tools/libs/light/libxl_domain.c   |  1 +
>   tools/libs/light/libxl_internal.h |  1 +
>   tools/xl/xl_cmdtable.c            |  2 ++
>   tools/xl/xl_migrate.c             | 13 ++++++++++---
>   8 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/CHANGELOG.md b/CHANGELOG.md
> index 225397f128..4f1da5d9e5 100644
> --- a/CHANGELOG.md
> +++ b/CHANGELOG.md
> @@ -16,6 +16,9 @@ The format is based on [Keep a 
> Changelog](https://keepachangelog.com/en/1.0.0/)
>        afterwards.
>   
>   ### Added
> + - On x86:
> +   - Optional LZ4 compression of the memory stream during migration
> +     (xl migrate --compress).
>   
>   ### Removed
>    - On Arm:
> diff --git a/docs/man/xl.1.pod.in b/docs/man/xl.1.pod.in
> index 51405f4e33..b0439231e2 100644
> --- a/docs/man/xl.1.pod.in
> +++ b/docs/man/xl.1.pod.in
> @@ -488,6 +488,14 @@ domain.
>   
>   Display huge (!) amount of debug information during the migration process.
>   
> +=item B<--compress>
> +
> +Compress the memory stream using LZ4 inside libxenguest before sending it
> +to the destination host. This can significantly reduce migration time on
> +slow transports, and reduce dom0 CPU pressure during host evacuations when
> +the migration data is subsequently encrypted or copied through pipes.
> +Requires liblz4 support in libxenguest on both hosts.
> +
>   =item B<-p>
>   
>   Leave the domain on the receive side paused after migration.
> diff --git a/tools/include/libxl.h b/tools/include/libxl.h
> index d294fb36bd..1114c8f8d1 100644
> --- a/tools/include/libxl.h
> +++ b/tools/include/libxl.h
> @@ -1922,6 +1922,7 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, 
> int fd,
>                            LIBXL_EXTERNAL_CALLERS_ONLY;
>   #define LIBXL_SUSPEND_DEBUG 1
>   #define LIBXL_SUSPEND_LIVE 2
> +#define LIBXL_SUSPEND_COMPRESS_LZ4 4
>   
I think these wants to be converted to (1<<N) form.

>   /*
>    * Only suspend domain, do not save its state to file, do not destroy it.
> diff --git a/tools/libs/light/libxl_dom_save.c 
> b/tools/libs/light/libxl_dom_save.c
> index d64fd64f2e..16b882ecf0 100644
> --- a/tools/libs/light/libxl_dom_save.c
> +++ b/tools/libs/light/libxl_dom_save.c
> @@ -245,6 +245,7 @@ void libxl__domain_save(libxl__egc *egc, 
> libxl__domain_save_state *dss)
>       const libxl_domain_type type = dss->type;
>       const int live = dss->live;
>       const int debug = dss->debug;
> +    const int compress_lz4 = dss->compress_lz4;
>       const libxl_domain_remus_info *const r_info = dss->remus;
>       libxl__srm_save_autogen_callbacks *const callbacks =
>           &dss->sws.shs.callbacks.save.a;
> @@ -269,7 +270,8 @@ void libxl__domain_save(libxl__egc *egc, 
> libxl__domain_save_state *dss)
>       if (rc) goto out;
>   
>       dss->xcflags = (live ? XCFLAGS_LIVE : 0)
> -          | (debug ? XCFLAGS_DEBUG : 0);
> +          | (debug ? XCFLAGS_DEBUG : 0)
> +          | (compress_lz4 ? XCFLAGS_COMPRESS_LZ4 : 0);
>   
>       /* Disallow saving a guest with vNUMA configured because migration
>        * stream does not preserve node information.
> diff --git a/tools/libs/light/libxl_domain.c b/tools/libs/light/libxl_domain.c
> index 37fcd92871..86458a69c0 100644
> --- a/tools/libs/light/libxl_domain.c
> +++ b/tools/libs/light/libxl_domain.c
> @@ -527,6 +527,7 @@ int libxl_domain_suspend(libxl_ctx *ctx, uint32_t domid, 
> int fd, int flags,
>       dss->type = type;
>       dss->live = flags & LIBXL_SUSPEND_LIVE;
>       dss->debug = flags & LIBXL_SUSPEND_DEBUG;
> +    dss->compress_lz4 = flags & LIBXL_SUSPEND_COMPRESS_LZ4;
>       dss->checkpointed_stream = LIBXL_CHECKPOINTED_STREAM_NONE;
>   
>       rc = libxl__fd_flags_modify_save(gc, dss->fd,
> diff --git a/tools/libs/light/libxl_internal.h 
> b/tools/libs/light/libxl_internal.h
> index b65e0064b9..ff7a334501 100644
> --- a/tools/libs/light/libxl_internal.h
> +++ b/tools/libs/light/libxl_internal.h
> @@ -3598,6 +3598,7 @@ struct libxl__domain_save_state {
>       libxl_domain_type type;
>       int live;
>       int debug;
> +    int compress_lz4;
>       int checkpointed_stream;
>       const libxl_domain_remus_info *remus;
>       /* private */
> diff --git a/tools/xl/xl_cmdtable.c b/tools/xl/xl_cmdtable.c
> index e48d5c5edc..aadca23a77 100644
> --- a/tools/xl/xl_cmdtable.c
> +++ b/tools/xl/xl_cmdtable.c
> @@ -174,6 +174,8 @@ const struct cmd_spec cmd_table[] = {
>         "-e              Do not wait in the background (on <host>) for the 
> death\n"
>         "                of the domain.\n"
>         "--debug         Print huge (!) amount of debug during the migration 
> process.\n"
> +      "--compress      Compress the memory stream using LZ4 in libxenguest.\n"
> +      "                Requires liblz4 support in libxenguest on both hosts.\n"
>         "-p              Do not unpause domain after migrating it.\n"
>         "-D              Preserve the domain id"
>       },
> diff --git a/tools/xl/xl_migrate.c b/tools/xl/xl_migrate.c
> index 4b4a379aa1..31a8cb4f32 100644
> --- a/tools/xl/xl_migrate.c
> +++ b/tools/xl/xl_migrate.c
> @@ -177,7 +177,7 @@ static void migrate_do_preamble(int send_fd, int recv_fd, 
> pid_t child,
>   }
>   
>   static void migrate_domain(uint32_t domid, int preserve_domid,
> -                           const char *rune, int debug,
> +                           const char *rune, int debug, int compress_lz4,
>                              const char *override_config_file)
>   {
>       pid_t child = -1;
> @@ -206,6 +206,8 @@ static void migrate_domain(uint32_t domid, int 
> preserve_domid,
>   
>       if (debug)
>           flags |= LIBXL_SUSPEND_DEBUG;
> +    if (compress_lz4)
> +        flags |= LIBXL_SUSPEND_COMPRESS_LZ4;
>       rc = libxl_domain_suspend(ctx, domid, send_fd, flags, NULL);
>       if (rc) {
>           fprintf(stderr, "migration sender: libxl_domain_suspend failed"
> @@ -538,10 +540,11 @@ int main_migrate(int argc, char **argv)
>       char *rune = NULL;
>       char *host;
>       int opt, daemonize = 1, monitor = 1, debug = 0, pause_after_migration = 0;
> -    int preserve_domid = 0;
> +    int preserve_domid = 0, compress_lz4 = 0;
>       static struct option opts[] = {
>           {"debug", 0, 0, 0x100},
>           {"live", 0, 0, 0x200},
> +        {"compress", 0, 0, 0x300},
>           COMMON_LONG_OPTS
>       };
>   
> @@ -571,6 +574,9 @@ int main_migrate(int argc, char **argv)
>       case 0x200: /* --live */
>           /* ignored for compatibility with xm */
>           break;
> +    case 0x300: /* --compress */
> +        compress_lz4 = 1;
> +        break;
>       }
>   
>       domid = find_domain(argv[optind]);
> @@ -602,7 +608,8 @@ int main_migrate(int argc, char **argv)
>                     pause_after_migration ? " -p" : "");
>       }
>   
> -    migrate_domain(domid, preserve_domid, rune, debug, config_filename);
> +    migrate_domain(domid, preserve_domid, rune, debug, compress_lz4,
> +                   config_filename);
>       return EXIT_SUCCESS;
>   }
>   
The rest looks good to me.

Teddy

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature