Integrate QCOW2 import-export #6573
Conversation
This patch allows to pass "qcow2" as a supported format when calling VDI export and import. Qcow_tool_wrapper is added as a helper that calls the Python script for export (conversion from raw to qcow2 stream) and the OCaml Qcow_stream library for import (conversion from qcow2 stream to raw). Signed-off-by: Guillaume <[email protected]> Signed-off-by: Andrii Sultanov <[email protected]>
…ons calling select With Qcow_stream being called directly from Qcow_tool_wrapper, Lwt runtime is now directly called from xapi. Signed-off-by: Andrii Sultanov <[email protected]>
Also add an mli file for qcow_tool_wrapper Signed-off-by: Andrii Sultanov <[email protected]>
| @@ -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_).*/) { | |||
There was a problem hiding this comment.
Is this something we want to permit? having lwt code running in the xapi process?
There was a problem hiding this comment.
that's what I'm asking in the PR message, yeah :(
There was a problem hiding this comment.
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
There was a problem hiding this comment.
What introduces Lwt here? I would also suspect that this leads into uncharted territory
There was a problem hiding this comment.
It's the receive call that uses the lwt-based library
There was a problem hiding this comment.
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).
|
I ok'ed this because we are not going to use this and expect Vates to see if the Lwt code causes a problem. |
|
If possible, I would like to hear @edwintorok's opinion on directly linking Lwt before merging this - it's his test I'm changing as well |
|
I'm closing this PR following Edwin's advice, will rework it to use the qcow-tool CLI to not infect the xapi process |
Final touches for qcow2 VDI import/export - add it as one of the supported formats and plumb through the calls to the python script for export and `qcow-stream` library for import via `qcow_tool_wrapper` and `qcow-stream-tool`. `qcow-stream-tool` is a thin wrapper executable over `qcow-stream`, so that xapi doesn't have to use Lwt code in its process (it's not thread-safe, see #6573 for the discussion on this) --- This was tested manually, confirmed to have the same behaviour as the previous version (#6573) --- This PR requires the following additions to the specfile: ```diff @@ -166,6 +172,7 @@ Requires: hwdata Requires: /usr/sbin/ssmtp Requires: stunnel >= 5.55 Requires: vhd-tool +Requires: qcow-stream-tool Requires: libffi Requires: busybox Requires: iproute @@ -397,9 +404,15 @@ This packages contains plugins registering to the RRD daemon and exposing variou %package -n vhd-tool Summary: Command-line tools for manipulating and streaming .vhd format files +%package -n qcow-stream-tool +Summary: Minimal CLI wrapper for qcow-stream + %description -n vhd-tool Simple command-line tools for manipulating and streaming .vhd format file. +%description -n qcow-stream-tool +Minimal CLI wrapper for qcow-stream + %package -n xcp-networkd Summary: Simple host network management service for the xapi toolstack Requires: ethtool @@ -1339,6 +1353,9 @@ done /opt/xensource/libexec/get_nbd_extents.py /opt/xensource/libexec/python_nbd_client.py +%files -n qcow-stream-tool +%{_bindir}/qcow-stream-tool + %files -n xcp-networkd %{_sbindir}/xcp-networkd %{_bindir}/networkd_db ```
Final touches for qcow2 import/export - add it as one of the supported formats and plumb through the calls to the python script for export and
qcow-streamlibrary for import viaqcow_tool_wrapper.Here I've decided to integrate qcow-stream directly, not through the CLI - this allows us to easily track progress of the transfer with a callback, and potentially get more information from the library (virtual size before starting the transfer by first parsing the header), but this means that the Lwt runtime is directly called in xapi (which required modifying the
test_selecttest) - is this bad tone? If so, I could convert to use the CLI instead, but this would require a few more dependencies in the xs-opam in addition to losing the benefits above.This was tested thoroughly over several weeks - the current xcp-ng qcow2 preview includes this and XenOrchestra's new backup code is relying on it as well.