Re: [PATCH v1 1/6] tools/migration: introduce PAGE_DATA_LZ4 stream record type

[Marcus Granado <marcus.granado@xxxxxxxxxx>]

On 21/07/2026 at 09:20, Teddy Astie wrote:
> Le 20/07/2026 à 17:51, Marcus Granado a écrit :
>> +PAGE_DATA_LZ4
>> +-------------
>> +
>> +A PAGE_DATA_LZ4 record carries exactly the same information as a
>> +PAGE_DATA record, but with the page contents LZ4-compressed.  The saver
>> +may emit it in place of a PAGE_DATA record when LZ4 compression has been
>> +requested.
>
> Would it be preferable to make this structure more generic, i.e
> PAGE_DATA_COMPRESSED, as I'm not sure it's wise to restrict to only LZ4
> (someone may want to add support for e.g zstd or another algorithm
> without having to change the format) ?

It should be possible to create a more generic structure like
PAGE_DATA_COMPRESSED instead of PAGE_DATA_LZ4, which accepts different
compression algorithms in extra new fields. I suspect the challenge here is
how generic to make it: I wonder if it wouldn't be equivalent and simpler to
instead add new record types (PAGE_DATA_ZSTD etc) in the future when
necessary, as a new algorithm needs new code on both ends anyway and reuses
the same per-page framing, so a generic record would not actually avoid a
code change. Perhaps we could adopt a PAGE_DATA_COMPRESSED_* family prefix
for the wire format doc, and rename the algorithm type PAGE_DATA_LZ4 ->
PAGE_DATA_COMPRESSED_LZ4, so that eg. in the future
PAGE_DATA_COMPRESSED_ZSTD etc reuses the same family document and is only a
new type id. This would be an interesting question for the maintainers of
libxenguest.

>> +            Each sub-block is a `uint16` little-endian length `clen`
>> +            followed by either an LZ4 block or a raw page.  When
>> +            `clen > 0`, page_data_lz4 is `clen` octets of a raw LZ4
>> +            block [...] When `clen == 0`, page_data_lz4 is
>> +            page_size octets of a raw, uncompressed page.
>
> The inline clen at the beginning of page_data_lz4 feels a bit odd to me,
> or at least, a bit inconsistent with how pfns are arranged. I think
> something like i.e
>
> 1. count and (reserved)
> 2. pfn[0] ... pfn[C-1]
> 3. clen[0] ... clen[N-1]
> 4. page_data_lz4[0] ... page_data_lz4[N-1]
>
> would be better.

Using an array clen[0..N-1] before the page_data_lz4[0..N-1] is neat and as
you said follows the existing pfn[0..C-1] pattern, and I think it would be
good to adopt it in v2.

> Though, aside that, I'm not sure having separate compressed block for
> each page is a good idea. Compression is more efficient when processing
> larger blocks, LZ4 documents a 64 KB deduplication window (of the past
> bytes) [1].

LZ4 expects the source buffer to be immutable for the duration of the
compression call. However, during live migration the guest is still running
and updating its pages concurrently with the compression. This mutable
source buffer can cause compressed back-references to be inconsistent in the
>4KB deduplication window (LZ4's match window reaches 64 KB, ie. up to 16
pages), resulting in corrupt decompressed pages on the destination.
Corrupted dirty pages would be resent and healed in the next iteration, but
corrupted clean (non-dirty) pages wouldn't. Indeed, I initially experienced
with compressing the whole 4MB batch and found the guests would crash on the
destination due to corrupted page contents after decompression. Compressing
individual 4KB pages avoids this back-reference inconsistency issue across
pages. Any inconsistency caused by a concurrent write inside an
individually-compressed 4KB page is not a problem, as it's marked dirty
anyway and will be resend in the next iteration, so the problem is only
present when compressing together >1 page.

I also tried staging: compress from a read-only copy of the whole 4 MB batch
to avoid the concurrent guest writes. That is safe, but it adds a second,
slower full memory pass per batch, which I suspect is better to avoid to
minimise memory bottlenecks in this high-throughput memory transfer
operation. Measured on identical buffers, per-page in general costs <1% of
the batch on ordinary compressible pages and is no worse on incompressible
pages (it stores them raw rather than expanding them); per-batch wins when a
page is duplicated within LZ4's 64 KB window, which per-page cannot dedup,
and in the measured scenarios the aggregate effect on a real migration is
not visible within the run-to-run variations when comparing per-batch
compression vs per-page compression, so in v1 I adopted the per-4K-page
compression.

> In my opinion, it may be preferable to have one large block with all the
> page datas (covering up to MAX_BATCH_SIZE=1024 pages) rather than having
> to uncompress individual blocks. Which in the end would remove the need
> to have individual clen[n].
>
> What do you think ?

Regarding using one large block (B=1), this would resurface the issue of the
cross-page back references problem, as the guest keeps updating the pages
while we compress them. Per-page independent blocks (B=N) make it safe to
compress directly from the live mapping. Right now we have B=N (the
zero-copy non-staged compression in v1), and we could extend it to allow B=1
(the cross-page compression with staging) in the future, something like:

1. count and (reserved)
2. pfn[0] ... pfn[C-1]
3. B
   if B==N: zero-copy, non-staged per-page compression units
   if B==1: staged per-batch compression unit
4. clen[0] ... clen[B-1]
5. cdata[0] ... cdata[B-1]

If we wanted to allow a generic 1<=B<=N, for B arbitrary compression units
for arbitrary blocks in each unit, then we'd probably need another array
plen[0..B-1] to map a compression unit to expected decompressed
cdata[0..B-1] size for the decompressor.