Skip to content

qcow_tool_wrapper: Add progress reporting for import and export - #7191

Open
last-genius wants to merge 3 commits into
xapi-project:26.1-lcmfrom
last-genius:asv/26.1-fix-percent
Open

qcow_tool_wrapper: Add progress reporting for import and export#7191
last-genius wants to merge 3 commits into
xapi-project:26.1-lcmfrom
last-genius:asv/26.1-fix-percent

Conversation

@last-genius

@last-genius last-genius commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Depends on xapi-project/xs-opam#771 being merged for progress reporting to be fixed completely.

Proceeds in three steps:

  1. consolidates vhd and qcow run_tool to use the same code (with the same progress reading logic)
  2. adds progress reporting to the qcow export side (python qcow2-to-stdout script)
  3. adds progress reporting to the qcow import side (above xs-opam PR + supplied callback)

@last-genius

Copy link
Copy Markdown
Contributor Author

I've tested this code on top of the xs-opam PR, manually verifying that the progress is reported correctly now.

I ran VDI and VM export/import tests for VHD and QCOW2 to make sure there aren't any regressions.

Comment thread ocaml/xapi/vhd_qcow_parsing.ml Outdated
Comment on lines +45 to +46
Xapi_stdext_pervasives.Pervasiveext.finally
(fun () ->

@psafont psafont Jul 22, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the patterns protect ~finally @@ fun () -> or let@ () = protect ~finally in after defining protect as let protect ~finally f = Pervasives.finally f finally will keep indentation and reduce the diff

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, together with another issue where waitpid and cleanup would only be called in the None arm of match pipe_read

@last-genius
last-genius force-pushed the asv/26.1-fix-percent branch from fd5170b to b5c1e04 Compare July 22, 2026 14:32
This removes the duplicated logic (Vhd_qcow_parsing.run_tool and
Vhd_tool_wrapper.run_vhd_tool were pretty much the same function with minor
differences).

Vhd_qcow_parsing.run_tool will now try to read back the progress percentages
reported over stdout if ~output_fd was not specified, following
Vhd_tool_wrapper.run_vhd_tool's behaviour

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>
Since progress reporting now takes up stdout, this requires passing the output
file as another file descriptor.

Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>
Tracking-reference: XCPNG-3562
Signed-off-by: Andrii Sultanov <andriy.sultanov@vates.tech>
@last-genius
last-genius force-pushed the asv/26.1-fix-percent branch from b5c1e04 to a700e9b Compare July 22, 2026 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants