Skip to content

Conversation

@last-genius
Copy link
Contributor

This allows it to proceed in parallel with the parse side, not deadlocking on the filled pipe.

This allows it to proceed in parallel with the parse side, not deadlocking on
the filled pipe.

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

@edwintorok edwintorok left a comment

Choose a reason for hiding this comment

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

Looks plausible, although there is some potential for some code cleanup in the future: those 2 snippets of code look almost identical.

I'll try creating a build and running some tests.

Copy link
Contributor

@lindig lindig left a comment

Choose a reason for hiding this comment

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

Looks safe to me.

@edwintorok
Copy link
Member

edwintorok commented Feb 9, 2026

For testing I'm using this commit and it reproduces the vm-export hang (without this PR):

commit d8b425c6315582ab4a4538320cb4ee37b64c406e (HEAD -> private/edvint/test-vhd, origin/private/edvint/test-vhd)
Author: Edwin Török <edwin.torok@citrix.com>
Date:   Mon Feb 9 15:43:10 2026 +0000

    TEST DO NOT MERGE: make JSON bigger

diff --git a/ocaml/vhd-tool/src/impl.ml b/ocaml/vhd-tool/src/impl.ml
index 530c915b8e..6aa73ce3cc 100644
--- a/ocaml/vhd-tool/src/impl.ml
+++ b/ocaml/vhd-tool/src/impl.ml
@@ -1170,6 +1170,7 @@ let stream_t common args ?(progress = no_progress_bar) () =

 let read_headers common source =
   let path = [Filename.dirname source] in
+  print_string (String.make 128000 ' ');

@edwintorok
Copy link
Member

With this PR export succeeds (atlhough not sure about error handling from the tool itself, but that'll become obvious if we fail to parse the JSON, and there is a fallback there to read the entire vhd instead then)

@edwintorok edwintorok marked this pull request as ready for review February 9, 2026 16:04
@edwintorok edwintorok added this pull request to the merge queue Feb 9, 2026
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 9, 2026
@edwintorok edwintorok added this pull request to the merge queue Feb 9, 2026
Merged via the queue into xapi-project:master with commit 420b584 Feb 9, 2026
16 checks passed
github-merge-queue bot pushed a commit that referenced this pull request Feb 9, 2026
This allows it to proceed in parallel with the parse side, not
deadlocking on the filled pipe.

Backport of #6901
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.

3 participants