Optimize data cluster format for VHD and QCOW by last-genius · Pull Request #6895 · xapi-project/xen-api

last-genius · 2026-02-06T13:08:03Z

mirage/ocaml-qcow#134 changes the type of the data structure containing info on allocated data clusters, returning allocated intervals instead of all the virtual cluster addresses. Change qcow2-to-stdout to the new interval-based format.

Add vhd-tool read_headers_interval command which also conforms to this new format, and change the parsing code in stream_vdi to accept both formats depending on a feature flag. Add cram tests verifying legacy format is preserved as-is.

I've ran vm export and vdi integrity quicktests and tested this extensively locally. The PR will only build once the new ocaml-qcow version is packaged into xs-opam, so keeping this as draft for now.

last-genius · 2026-02-18T09:47:08Z

Opened the corresponding xs-opam PR: xapi-project/xs-opam#755

lindig · 2026-02-18T14:03:30Z

ocaml/xapi/vhd_qcow_parsing.ml

+          | x :: y :: _ ->
+              (to_int x, to_int y)
+          | _ ->
+              raise (Invalid_argument "Invalid JSON")


You might want to report the json.

It can be rather large and pollute the logs... This shouldn't happen unless you have a version incompatibility, really

last-genius · 2026-02-19T10:37:56Z

Changed the approach:

Refactored the QCOW-only side to the interval-based format (qcow2-to-stdout.py + new interface in ocaml-qcow)
Added the interval-based format to vhd-tool alongside "legacy" format
Added a vhd_legacy_blocks_format feature flag to control which format stream_vdi uses for VHD and QCOW. It's defaulted to legacy.
Added cram tests to verify vhd-tool read_headers legacy format is preserved across this refactoring.

This way, XS (which only uses VHD) can keep using the legacy format until they can ensure the new format didn't accidentally break anything else. XCP-ng will flip the feature flag to make VHD consistent with QCOW - we will use the interval-based format.

ocaml/vhd-tool/cli/read_headers.t

psafont · 2026-03-05T10:46:03Z

dune-project

  cohttp
  cohttp-lwt
  conf-libssl
+  (conf-qemu-img :with-test)


There's no conf package for qemu-img, even though for some reason I thought it existed.

Another way of configuring it is:

depexts: [ ["qemu-img"] {with-test} ]

I added it last year, seems to still be there: https://github.com/ocaml/opam-repository/blob/master/packages/conf-qemu-img/conf-qemu-img.1/opam

I don't see it in xs-opam, though

I added it in xapi-project/xs-opam#755

Is this waiting for a review from Edwin, then? I think it was him that raised concerns

Yes, CC @edwintorok

This allows to switch on the more efficient interval format later. (QCOW always uses the new format) Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

This is just an easy way to make sure the semantics are preserved in any future refactorings, without having to run full VHD exports. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

This command returns a more efficient representation of allocated clusters (when compared to read_headers), utilizing a sparse interval format instead of returning every single allocated cluster. This is the more efficient option, decreasing the filesize and memory usage in vhd-tool, but it's currently under a feature flag, so it's added as a new command instead of replacing read_headers immediately. Cram test for read_headers is still passing, so this refactoring has preserved the legacy format. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

Since the runtime feature flag vhd_legacy_blocks_format determines which block format is used to describe allocated VHD clusters, this requires duplicate parse_header_interval functions for VHD and QCOW. The right functions are selected in stream_vdi based on the feature flag. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

…er allocation Instead of using a set with every individual allocated cluster index as a member, use a sorted list of intervals to verify if cluster is allocated - this uses much less memory and directly follows from the JSON format qcow-stream-tool and vhd-tool output now. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

…_clusters nonzero_clusters no longer contain every single allocated cluster and instead are intervals of allocated clusters. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

last-genius · 2026-04-09T08:51:48Z

I've switched approaches after more thorough testing.

Now qemu-img is used instead of ocaml-qcow to determine allocated clusters (it's much better with edge cases) - this is the new qcow_tool_wrapper: Call qemu-img instead of qcow-stream-tool commit. qemu-img's format is still interval-based, so that rework is still needed.

This (along with vhd-tool using qemu-img in its tests) means we need to package qemu-img for LCM. Is this viable/desirable?

I've dropped read_headers in qcow-stream-tool which means there's no longer a breaking change in the ocaml-qcow interface used by xapi, though there's still the new conf-qemu-img dependency that requires coordination between xs-opam and xapi PRs.

I've fixed more bugs in preceding commits (two in stream_vdi and one in qcow2-to-stdout)

lindig · 2026-04-09T09:46:47Z

ocaml/libs/vhd/vhd_format/f.ml

+                 else
+                   match left_block with
+                   | Some x ->
+                       (`List [`Int x; `Int (i - 1)] :: acc, None)


Is the JSON our own design or do we need to conform to something? If it's our own choice, I would prefer an object with two entries over a list if this JSON is consumed by software. If it's just for display and never parsed it's fine to do it this way because it is more compact.

it is our design, it's parsed in vhd_qcow_parsing. this two-element list represents the start and the end of the interval, so I don't see it expanding to more elements (which could be a reason to turn it into an object). we still want the compactness, though

lindig · 2026-04-09T09:52:13Z

ocaml/xapi/stream_vdi.ml

+                  (cluster_size + chunk_size - 1) / chunk_size
+                in
+                (* Iterate over allocated intervals, copying every cluster inside *)
+                let _ =


Is a fold the right way if we ignore the result?

couldn't think of a nicer way to thread through a value that's updated on every iteration (without refs)

This proves much more reliable than code based on ocaml-qcow. Since qemu-img has a different format (with the needed information spread across two files resulting from calls to 'qemu-img info' and 'qemu-img map'), change the parsing code in vhd_qcow_parsing and qcow2-to-stdout. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

qemu-img is now used to determine allocated clusters, so this command is no longer needed. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

last-genius · 2026-04-09T10:36:14Z

Apparently qemu-img is already packaged (by qemu to /usr/lib64/xen/bin/qemu-img), I've changed xapi_globs accordingly.

psafont

We need to coordinate the merge and release with xs-opam.

xs-opam's PR should be merged first.
Then pass the CI here.
Then merge this, and then we can make a release in xs-opam.

As usual with the breaking updates, this will break the CI in all PRs and feature branches, triggering a rebase.

Since this is done for LCM, I don't think there will be much of an impact, but this will happening when merging to master

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

last-genius · 2026-04-10T07:58:45Z

I've dropped the conf-qemu-img dependency from xapi, kept it in vhd-tool. xapi's spec file already has a dependency on qemu.

last-genius mentioned this pull request Feb 18, 2026

Update ocaml-qcow to 0.13.0 xapi-project/xs-opam#755

Open

last-genius marked this pull request as ready for review February 18, 2026 09:47

lindig approved these changes Feb 18, 2026

View reviewed changes

last-genius force-pushed the asv/8.3-sparse-vhd-qcow branch from d41347a to bcf47a8 Compare February 19, 2026 10:33

psafont approved these changes Mar 4, 2026

View reviewed changes

ocaml/vhd-tool/cli/read_headers.t Show resolved Hide resolved

psafont approved these changes Mar 5, 2026

View reviewed changes

psafont reviewed Mar 5, 2026

View reviewed changes

last-genius mentioned this pull request Mar 9, 2026

Switch to optimized data cluster format for VHD and QCOW xcp-ng-rpms/xapi#122

Merged

last-genius force-pushed the asv/8.3-sparse-vhd-qcow branch from d431ca9 to b2b80aa Compare April 9, 2026 08:39

last-genius added 8 commits April 9, 2026 08:46

xapi_globs: Add vhd_legacy_blocks_format feature flag

1aa52ef

This allows to switch on the more efficient interval format later. (QCOW always uses the new format) Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

vhd-tool: Add cram tests for read_headers

627f321

This is just an easy way to make sure the semantics are preserved in any future refactorings, without having to run full VHD exports. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

vhd-tool: Add read_headers_interval to the cram test

6a31727

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

python3/qcow2-to-stdout: Switch to sparse interval format for nonzero…

a52b126

…_clusters nonzero_clusters no longer contain every single allocated cluster and instead are intervals of allocated clusters. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

qcow_tool_wrapper: Add note on using header information from VHD files

f9db34b

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

last-genius force-pushed the asv/8.3-sparse-vhd-qcow branch from b2b80aa to 581edae Compare April 9, 2026 08:47

psafont requested review from lindig and psafont April 9, 2026 09:28

lindig approved these changes Apr 9, 2026

View reviewed changes

last-genius added 2 commits April 9, 2026 09:55

qcow-stream-tool: Drop read_headers

829c018

qemu-img is now used to determine allocated clusters, so this command is no longer needed. Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

last-genius force-pushed the asv/8.3-sparse-vhd-qcow branch from 581edae to d1beba5 Compare April 9, 2026 10:34

psafont approved these changes Apr 9, 2026

View reviewed changes

dune-project: Add conf-qemu-img dependency to vhd-tool

15afce2

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>

last-genius force-pushed the asv/8.3-sparse-vhd-qcow branch from d1beba5 to 15afce2 Compare April 10, 2026 07:58

Conversation

last-genius commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

last-genius commented Feb 18, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

last-genius commented Feb 19, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

last-genius commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

last-genius Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

last-genius commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

psafont left a comment

Choose a reason for hiding this comment

Uh oh!

last-genius commented Apr 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

last-genius commented Feb 6, 2026 •

edited

Loading

last-genius commented Apr 9, 2026 •

edited

Loading

last-genius Apr 9, 2026 •

edited

Loading

last-genius commented Apr 9, 2026 •

edited

Loading