Libguestfs - Oct 2015 - Re: [PATCH] v2v: virtio-win: include *.dll too

Windows QXL drivers include also qxldd.dll which used to get filtered
out and not copied over into the guest.  As a result QXL driver failed
to install due to a missing file.

Correct that, and update the tests accordingly.

Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
---
 v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll | 1 +
 v2v/test-v2v-in-place.sh                        | 1 +
 v2v/test-v2v-virtio-win-iso.sh                  | 1 +
 v2v/test-v2v-windows-conversion.sh              | 1 +
 v2v/utils.ml                                    | 2 +-
 5 files changed, 5 insertions(+), 1 deletion(-)
 create mode 100644 v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll

diff --git a/v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll
b/v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll
new file mode 100644
index 0000000..ff43465
--- /dev/null
+++ b/v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll
@@ -0,0 +1 @@
+This is a fake qxldd.dll, used for testing only
diff --git a/v2v/test-v2v-in-place.sh b/v2v/test-v2v-in-place.sh
index f685ddb..1e9972d 100755
--- a/v2v/test-v2v-in-place.sh
+++ b/v2v/test-v2v-in-place.sh
@@ -108,6 +108,7 @@ for drv in netkvm qxl vioscsi viostor; do
         mktest "is-file \"$virtio_dir/$drv.$sfx\"" true
     done
 done
+mktest "is-file \"$virtio_dir/qxldd.dll\"" true
 
 guestfish --ro -a "$img" -i < "$script" >
"$response"
 diff -u "$expected" "$response"
diff --git a/v2v/test-v2v-virtio-win-iso.sh b/v2v/test-v2v-virtio-win-iso.sh
index 090025c..e4228c1 100755
--- a/v2v/test-v2v-virtio-win-iso.sh
+++ b/v2v/test-v2v-virtio-win-iso.sh
@@ -103,6 +103,7 @@ for drv in netkvm qxl vioscsi viostor; do
         mktest "is-file \"$virtio_dir/$drv.$sfx\"" true
     done
 done
+mktest "is-file \"$virtio_dir/qxldd.dll\"" true
 
 guestfish --ro -a "$d/windows-sda" -i < "$script" >
"$response"
 diff -u "$expected" "$response"
diff --git a/v2v/test-v2v-windows-conversion.sh
b/v2v/test-v2v-windows-conversion.sh
index d086bc4..80b7096 100755
--- a/v2v/test-v2v-windows-conversion.sh
+++ b/v2v/test-v2v-windows-conversion.sh
@@ -97,6 +97,7 @@ for drv in netkvm qxl vioscsi viostor; do
         mktest "is-file \"$virtio_dir/$drv.$sfx\"" true
     done
 done
+mktest "is-file \"$virtio_dir/qxldd.dll\"" true
 
 guestfish --ro -a "$d/windows-sda" -i < "$script" >
"$response"
 diff -u "$expected" "$response"
diff --git a/v2v/utils.ml b/v2v/utils.ml
index 2a668ec..d65bb94 100644
--- a/v2v/utils.ml
+++ b/v2v/utils.ml
@@ -179,7 +179,7 @@ let virtio_iso_path_matches_guest_os path inspect      in
 
     (* Skip files without specific extensions. *)
-    let extensions = ["cat"; "inf"; "pdb";
"sys"] in
+    let extensions = ["cat"; "dll"; "inf";
"pdb"; "sys"] in
     if not (List.mem extension extensions) then raise Not_found;
 
     (* Using the full path, work out what version of Windows
-- 
2.4.3

On Mon, Oct 26, 2015 at 09:00:03PM +0300, Roman Kagan
wrote:
> Windows QXL drivers include also qxldd.dll which used to get filtered
> out and not copied over into the guest.  As a result QXL driver failed
> to install due to a missing file.
> 
> Correct that, and update the tests accordingly.
> 
> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
> ---
>  v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll | 1 +
>  v2v/test-v2v-in-place.sh                        | 1 +
>  v2v/test-v2v-virtio-win-iso.sh                  | 1 +
>  v2v/test-v2v-windows-conversion.sh              | 1 +
>  v2v/utils.ml                                    | 2 +-
>  5 files changed, 5 insertions(+), 1 deletion(-)
>  create mode 100644 v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll
> 
> diff --git a/v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll
b/v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll
> new file mode 100644
> index 0000000..ff43465
> --- /dev/null
> +++ b/v2v/fake-virtio-win/drivers/i386/Win7/qxldd.dll
> @@ -0,0 +1 @@
> +This is a fake qxldd.dll, used for testing only
> diff --git a/v2v/test-v2v-in-place.sh b/v2v/test-v2v-in-place.sh
> index f685ddb..1e9972d 100755
> --- a/v2v/test-v2v-in-place.sh
> +++ b/v2v/test-v2v-in-place.sh
> @@ -108,6 +108,7 @@ for drv in netkvm qxl vioscsi viostor; do
>          mktest "is-file \"$virtio_dir/$drv.$sfx\""
true
>      done
>  done
> +mktest "is-file \"$virtio_dir/qxldd.dll\"" true
>  
>  guestfish --ro -a "$img" -i < "$script" >
"$response"
>  diff -u "$expected" "$response"
> diff --git a/v2v/test-v2v-virtio-win-iso.sh
b/v2v/test-v2v-virtio-win-iso.sh
> index 090025c..e4228c1 100755
> --- a/v2v/test-v2v-virtio-win-iso.sh
> +++ b/v2v/test-v2v-virtio-win-iso.sh
> @@ -103,6 +103,7 @@ for drv in netkvm qxl vioscsi viostor; do
>          mktest "is-file \"$virtio_dir/$drv.$sfx\""
true
>      done
>  done
> +mktest "is-file \"$virtio_dir/qxldd.dll\"" true
>  
>  guestfish --ro -a "$d/windows-sda" -i < "$script"
> "$response"
>  diff -u "$expected" "$response"
> diff --git a/v2v/test-v2v-windows-conversion.sh
b/v2v/test-v2v-windows-conversion.sh
> index d086bc4..80b7096 100755
> --- a/v2v/test-v2v-windows-conversion.sh
> +++ b/v2v/test-v2v-windows-conversion.sh
> @@ -97,6 +97,7 @@ for drv in netkvm qxl vioscsi viostor; do
>          mktest "is-file \"$virtio_dir/$drv.$sfx\""
true
>      done
>  done
> +mktest "is-file \"$virtio_dir/qxldd.dll\"" true
>  
>  guestfish --ro -a "$d/windows-sda" -i < "$script"
> "$response"
>  diff -u "$expected" "$response"
> diff --git a/v2v/utils.ml b/v2v/utils.ml
> index 2a668ec..d65bb94 100644
> --- a/v2v/utils.ml
> +++ b/v2v/utils.ml
> @@ -179,7 +179,7 @@ let virtio_iso_path_matches_guest_os path inspect > 
in
>  
>      (* Skip files without specific extensions. *)
> -    let extensions = ["cat"; "inf"; "pdb";
"sys"] in
> +    let extensions = ["cat"; "dll"; "inf";
"pdb"; "sys"] in
>      if not (List.mem extension extensions) then raise Not_found;
>  
>      (* Using the full path, work out what version of Windows
Looks good.  I'll push this later today.

Thanks, Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On Mon, Oct 26, 2015 at 09:00:03PM +0300, Roman Kagan
wrote:
> Windows QXL drivers include also qxldd.dll which used to get filtered
> out and not copied over into the guest.  As a result QXL driver failed
> to install due to a missing file.
>      (* Skip files without specific extensions. *)
> -    let extensions = ["cat"; "inf"; "pdb";
"sys"] in
> +    let extensions = ["cat"; "dll"; "inf";
"pdb"; "sys"] in
Actually no, this part is (may be) wrong.

This causes WdfCoInstaller*.dll from the ISO to be copied in.  It's
not clear to me whether these files are needed - I suspect not.

It also causes the tests to fail, although only when you have
installed the optional ocaml oUnit2 package:

----------------------------------------------------------------------
Error: virt-v2v:3:Utils.virtio_iso_path_matches_guest_os.

File
"/home/rjones/d/libguestfs/v2v/oUnit-virt-v2v-choo.home.annexia.org#01.log",
line 8, characters 1-1:
Error: virt-v2v:3:Utils.virtio_iso_path_matches_guest_os (in the log).

Raised at file "src/oUnitAssert.ml", line 45, characters 8-27
Called from file "list.ml", line 73, characters 12-15
Called from file "list.ml", line 73, characters 12-15
Called from file "src/oUnitRunner.ml", line 46, characters 13-26

path Balloon/2k12/amd64/WdfCoInstaller01011.dll should not match win2k12_64
expected: false but got: true
----------------------------------------------------------------------

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

On Tue, Oct 27, 2015 at 09:12:41AM +0000, Richard W.M. Jones
wrote:
> On Mon, Oct 26, 2015 at 09:00:03PM +0300, Roman Kagan wrote:
> > Windows QXL drivers include also qxldd.dll which used to get filtered
> > out and not copied over into the guest.  As a result QXL driver failed
> > to install due to a missing file.
> >      (* Skip files without specific extensions. *)
> > -    let extensions = ["cat"; "inf";
"pdb"; "sys"] in
> > +    let extensions = ["cat"; "dll";
"inf"; "pdb"; "sys"] in
> 
> Actually no, this part is (may be) wrong.
> 
> This causes WdfCoInstaller*.dll from the ISO to be copied in.  It's
> not clear to me whether these files are needed - I suspect not.
Yes it is, see the corresponding *.inf where it's mentioned in the
corresponding CopyFiles directive (and yes, I just verified that with
that file missing the balloon driver installation fails with 'file not
found' error).

As a matter of fact I tend to think that filtering of files by suffix is
a bad idea in general: the packager of the drivers should be free to
ship any files he wants, including docs and text notes.  It's only the
path that should match against the guest OS flavor and that really
matters.  E.g. the balloon driver directory also contains
blnsvr.exe (and its .pdb) which is a usermode service responsible for
feeding the balloon device with the memory stats; it's not a part of the
driver per se but it's currently bundled with it.  (I'm yet to figure
out who's responsible for installing it).
> It also causes the tests to fail, although only when you have
> installed the optional ocaml oUnit2 package:
That's a problem with the test then (and no I didn't have that package
installed so my make -C v2v check passed).

Roman.