Re: [Libguestfs] [PATCH 0/4] v2v: simplify driver copying from virtio-win iso
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.
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.
- Calling String.lowercase is unsafe (because the function is just
broken in OCaml) and unnecessary. Just remove it.
- (Stylistic) You don't need parentheses around if conditionals.
- (Stylistic) You don't need parentheses around the arguments of &&
- (Stylistic) You don't need parentheses around the arguments of //
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines. Supports shell scripting,
bindings from many languages. http://libguestfs.org