On Thu, Oct 01, 2015 at 07:09:02PM +0300, Roman Kagan wrote:
On Thu, Oct 01, 2015 at 04:22:14PM +0100, Richard W.M. Jones wrote:
On Thu, Oct 01, 2015 at 06:04:03PM +0300, Roman Kagan wrote:
On Mon, Aug 10, 2015 at 06:55:28PM +0300, Roman Kagan wrote:
Libguestfs supports passing an ISO image as a source of virtio windows drivers to v2v.
That support, however, looks too heavy-weight: in order to access those drivers, a separate guestfs handle is created (and thus a new emulator process is started), which runs until v2v completes.
This series attempts to make it simpler and lighter-weight, by making the relevant code more local, and by hot-adding the image into the main guestfs handle.
Roman Kagan (4): v2v: drop useless forced gc v2v: consolidate virtio-win file copying v2v: copy virtio drivers without guestfs handle leak v2v: reuse main guestfs for virtio win drivers iso
v2v/convert_windows.ml | 184 ++++++++++++++++++++-------------------- v2v/utils.ml | 224 ++++++++++++++++--------------------------------- v2v/v2v.ml | 8 -- 3 files changed, 163 insertions(+), 253 deletions(-)
I just realized that, although (at least two of) these patches got "likely ACK", they are not in the libguestfs tree.
The patches still apply to master as of today.
Is there anything I can do to get them merged?
Actually better to post them again so I can review them again.
OK will do.
I spotted a few problems
- There's a missing pair of parentheses around the then clause in:
g2#mount_ro "/dev/sda" vio_root; let paths = g2#find vio_root in Array.iter ( fun path -> let source = vio_root // path in if ((g2#is_file source ~followsymlinks:false) && (match_vio_path_with_os path inspect.i_arch inspect.i_major_version inspect.i_minor_version inspect.i_product_variant)) then ^^^
which makes me think this code can't possibly work.
Indeed. It failed neither in compilation nor in tests, though. Looks like I need to patch v2v/test-v2v-windows-conversion.sh, too.
When I replied I didn't already remember for certain how I verified that the code did achieve its goal; however now that I have a test for it (submitted today, needs fixing the stylistic comments but working otherwise) I claim that it passes with this code, i.e. all the drivers do get copied into the guest upon conversion. So this code does work as intended; being totally clueless in OCaml I'd appreciate being explained -- how?
- Calling String.lowercase is unsafe (because the function is just broken in OCaml) and unnecessary. Just remove it.
I can't figure out why it's unnecessary and how to remove it. The only place where String.lowercase is called is in match_vio_path_with_os() which is basically unchanged in the patch, it's only factored out into a separate function. It tries to do case-insensitive comparison of the path elements to constant strings; is there a way to do it other than downcasing both sides of the comparison? Thanks, Roman.