Skip to content

Integrate QCOW2 import-export #6573

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Integrate QCOW2 import-export #6573

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
93eb72e
{export,import}_raw_vdi: add qcow as supported format
last-genius May 1, 2025
aebce65
tests/unix_select: Add Lwt_engine__fun to the list of expected functi…
last-genius May 7, 2025
3ad070b
export_raw_vdi: Add support for differential QCOW2 export with base
last-genius May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ocaml/tests/unix_select.gawk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ END {
for (idx in CALLS[SYM]) {
caller = CALLS[SYM][idx];
print "--"
if (caller ~ /caml(Thread|Unix__fun_).*/) {
if (caller ~ /caml(Thread|Unix__fun_|Lwt_engine__fun_).*/) {
Copy link
Member

@psafont psafont Jul 4, 2025

Choose a reason for hiding this comment

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

Is this something we want to permit? having lwt code running in the xapi process?

Copy link
Contributor Author

@last-genius last-genius Jul 4, 2025

Choose a reason for hiding this comment

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

that's what I'm asking in the PR message, yeah :(

Copy link
Member

@psafont psafont Jul 4, 2025

Choose a reason for hiding this comment

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

I think xapi has not linked against lwt on purpose, because of issues due to threading. I'm not sure what the concerns or issues were, exactly

Copy link
Contributor

@lindig lindig Jul 7, 2025

Choose a reason for hiding this comment

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

What introduces Lwt here? I would also suspect that this leads into uncharted territory

Copy link
Member

@psafont psafont Jul 7, 2025

Choose a reason for hiding this comment

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

It's the receive call that uses the lwt-based library

Copy link
Contributor

@edwintorok edwintorok Jul 15, 2025

Choose a reason for hiding this comment

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

Don't combine Lwt with multi-threaded code: Lwt itself is not thread-safe.
There are ways to make it work (e.g. with https://ocaml.org/p/lwt/5.9.1/doc/lwt.unix/Lwt_preemptive/index.html and https://ocaml.org/p/lwt/5.9.1/doc/lwt.unix/Lwt_unix/index.html#val-send_notification), but the type system won't help you find what is and isn't thread safe and is best avoided.

You either fully convert an application to Lwt (which means monadifying everything... not worth it for XAPI, when in OCaml 5 we could use effects instead), or very carefully isolate Lwt code. It is best to just avoid mixing Lwt with multithreaded code though.
(Lwt internally uses multiple threads, but only for C code).

# direct calls from these functions to unix_select are expected
print caller "[expected]"
} else {
Expand Down