Libguestfs - Feb 2017 - [PATCH v2 0/3] Fix OVA import with libvirt backend

v2:
- 1/3: same as in previous version
- 2/3: add check for libvirt version
- 3/3: restore the disabled test

The libvirt patch [2] that should fix our problem with 'raw' driver that
is mentioned in [1] was merged.

To realy fix things, there is at least one thing we have to change on our side.
We have to be explicit about the driver in the JSON we generate.

[1] https://www.redhat.com/archives/libguestfs/2017-February/msg00101.html
[2] https://www.redhat.com/archives/libvir-list/2017-February/msg00575.html

Tomáš Golembiovský (3):
  v2v: ova: fix generated JSON for libvirt support
  v2v: ova: check libvirt version before OVA import
  v2v: tests: restore disabled OVA test

 v2v/Makefile.am                         |  4 ++--
 v2v/input_ova.ml                        |  4 +++-
 v2v/test-v2v-i-ova-subfolders.expected2 |  2 +-
 v2v/test-v2v-i-ova-tar.expected2        |  2 +-
 v2v/test-v2v-i-ova-two-disks.expected2  |  4 ++--
 v2v/test-v2v-i-ova.sh                   |  4 +---
 v2v/utils.ml                            | 14 ++++++++++++++
 v2v/utils.mli                           |  5 +++++
 8 files changed, 29 insertions(+), 10 deletions(-)

-- 
2.11.1

We have to be explicit about the drivers in backing file JSON in order
for libvirt to work.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 v2v/input_ova.ml                        | 1 +
 v2v/test-v2v-i-ova-subfolders.expected2 | 2 +-
 v2v/test-v2v-i-ova-tar.expected2        | 2 +-
 v2v/test-v2v-i-ova-two-disks.expected2  | 4 ++--
 4 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index fff1c22cc..5e63b605e 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -375,6 +375,7 @@ object
                   "offset", JSON.Int64 offset;
                   "size", JSON.Int64 size;
                   "file", JSON.Dict [
+                    "driver", JSON.String "file";
                     "filename", JSON.String ova]
                   ]
                 ] in
diff --git a/v2v/test-v2v-i-ova-subfolders.expected2
b/v2v/test-v2v-i-ova-subfolders.expected2
index ef2993ed6..5029760c9 100644
--- a/v2v/test-v2v-i-ova-subfolders.expected2
+++ b/v2v/test-v2v-i-ova-subfolders.expected2
@@ -10,7 +10,7 @@ hypervisor type: vmware
           video: 
           sound: 
 disks:
-	json:{ "file": { "driver": "raw",
"offset": x, "size": 10240, "file": {
"filename": "test.ova" } } } (vmdk) [scsi]
+	json:{ "file": { "driver": "raw",
"offset": x, "size": 10240, "file": {
"driver": "file", "filename": "test.ova"
} } } (vmdk) [scsi]
 removable media:
 	CD-ROM [ide] in slot 0
 NICs:
diff --git a/v2v/test-v2v-i-ova-tar.expected2 b/v2v/test-v2v-i-ova-tar.expected2
index 200e32a70..a97a7149c 100644
--- a/v2v/test-v2v-i-ova-tar.expected2
+++ b/v2v/test-v2v-i-ova-tar.expected2
@@ -10,7 +10,7 @@ hypervisor type: vmware
           video: 
           sound: 
 disks:
-	json:{ "file": { "driver": "raw",
"offset": x, "size": 10240, "file": {
"filename": "test-tar.ova" } } } (vmdk) [scsi]
+	json:{ "file": { "driver": "raw",
"offset": x, "size": 10240, "file": {
"driver": "file", "filename":
"test-tar.ova" } } } (vmdk) [scsi]
 removable media:
 	CD-ROM [ide] in slot 0
 NICs:
diff --git a/v2v/test-v2v-i-ova-two-disks.expected2
b/v2v/test-v2v-i-ova-two-disks.expected2
index cf4cea50b..621ace84b 100644
--- a/v2v/test-v2v-i-ova-two-disks.expected2
+++ b/v2v/test-v2v-i-ova-two-disks.expected2
@@ -10,8 +10,8 @@ hypervisor type: vmware
           video: 
           sound: 
 disks:
-	json:{ "file": { "driver": "raw",
"offset": x, "size": 10240, "file": {
"filename": "test.ova" } } } (vmdk) [scsi]
-	json:{ "file": { "driver": "raw",
"offset": x, "size": 102400, "file": {
"filename": "test.ova" } } } (vmdk) [scsi]
+	json:{ "file": { "driver": "raw",
"offset": x, "size": 10240, "file": {
"driver": "file", "filename": "test.ova"
} } } (vmdk) [scsi]
+	json:{ "file": { "driver": "raw",
"offset": x, "size": 102400, "file": {
"driver": "file", "filename": "test.ova"
} } } (vmdk) [scsi]
 removable media:
 	CD-ROM [ide] in slot 0
 NICs:
-- 
2.11.1

Libvirt < 3.1.0 lacks enough support for json: pseudo-URLs. Notably it
does not allow use of "raw" driver that we need.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 v2v/Makefile.am  |  4 ++--
 v2v/input_ova.ml |  3 ++-
 v2v/utils.ml     | 14 ++++++++++++++
 v2v/utils.mli    |  5 +++++
 4 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index ffd514426..29ce0ea72 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -63,10 +63,10 @@ SOURCES_MLI = \
 SOURCES_ML = \
 	types.ml \
 	uefi.ml \
-	utils.ml \
 	name_from_disk.ml \
 	vCenter.ml \
 	domainxml.ml \
+	utils.ml \
 	DOM.ml \
 	changeuid.ml \
 	OVF.ml \
@@ -175,9 +175,9 @@ virt_v2v_copy_to_local_CFLAGS = \
 
 COPY_TO_LOCAL_BOBJECTS = \
 	uefi.cmo \
-	utils.cmo \
 	vCenter.cmo \
 	domainxml.cmo \
+	utils.cmo \
 	copy_to_local.cmo
 COPY_TO_LOCAL_XOBJECTS = $(COPY_TO_LOCAL_BOBJECTS:.cmo=.cmx)
 
diff --git a/v2v/input_ova.ml b/v2v/input_ova.ml
index 5e63b605e..7044a6bb8 100644
--- a/v2v/input_ova.ml
+++ b/v2v/input_ova.ml
@@ -91,7 +91,8 @@ object
         match detect_file_type ova with
         | `Tar ->
           (* Normal ovas are tar file (not compressed). *)
-          if qemu_img_supports_offset_and_size () then (
+          if qemu_img_supports_offset_and_size () &&
+              libvirt_supports_json_raw_driver () then (
             (* In newer QEMU we don't have to extract everything.
              * We can access disks inside the tar archive directly.
              *)
diff --git a/v2v/utils.ml b/v2v/utils.ml
index 88c43eb63..20510208c 100644
--- a/v2v/utils.ml
+++ b/v2v/utils.ml
@@ -119,6 +119,20 @@ let qemu_img_supports_offset_and_size ()    debug
"qemu-img supports \"offset\" and \"size\" in json
URLs: %b" r;
   r
 
+(* Libvirt < 3.1.0 lacks enough support for json: pseudo-URLs we use as
backing
+ * files, when importing OVAs directly without extracting them first.
+ *)
+let libvirt_supports_json_raw_driver () +  let libguestfs_backend =
(open_guestfs ())#get_backend () in
+  let libguestfs_backend, _ = String.split ":" libguestfs_backend in
+  if libguestfs_backend = "libvirt" then (
+    let sup = Domainxml.libvirt_get_version () >= (3, 1, 0) in
+    debug "libvirt supports  \"raw\" driver in json URL:
%B" sup;
+    sup
+  )
+  else
+    true
+
 let find_file_in_tar tar filename    let lines = external_command (sprintf
"tar tRvf %s" (Filename.quote tar)) in
   let rec loop lines diff --git a/v2v/utils.mli b/v2v/utils.mli
index b75baa7f6..77ff24ea5 100644
--- a/v2v/utils.mli
+++ b/v2v/utils.mli
@@ -55,6 +55,11 @@ val qemu_img_supports_offset_and_size : unit -> bool
 (** Return true iff [qemu-img] supports the ["offset"] and
["size"]
     parameters to open a subset of a file. *)
 
+val libvirt_supports_json_raw_driver : unit -> bool
+(** Return true if [libvirt] supports ["json:"] pseudo-URLs and
accepts the
+    ["raw"] driver. Function also returns true if [libvirt] backend
is not
+    used. *)
+
 val find_file_in_tar : string -> string -> int64 * int64
 (** [find_file_in_tar tar filename] looks up file in [tar] archive and returns
     a tuple containing at which byte it starts and how long the file is.
-- 
2.11.1

We are redy to restore the disabled test. The relevant libvirt fixes
should be available in 3.1.0.

Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com>
---
 v2v/test-v2v-i-ova.sh | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/v2v/test-v2v-i-ova.sh b/v2v/test-v2v-i-ova.sh
index 7a33b6c9c..e36c68d0b 100755
--- a/v2v/test-v2v-i-ova.sh
+++ b/v2v/test-v2v-i-ova.sh
@@ -25,9 +25,7 @@ skip_if_skipped
 skip_if_backend uml
 skip_unless_phony_guest windows.img
 
-# XXX Remove when we fix this.  See:
-# https://www.redhat.com/archives/libguestfs/2017-February/msg00101.html
-skip_if_backend libvirt
+skip_unless_libvirt_minimum_version 3 1 0
 
 export VIRT_TOOLS_DATA_DIR="$top_srcdir/test-data/fake-virt-tools"
 export VIRTIO_WIN="$top_srcdir/test-data/fake-virtio-win"
-- 
2.11.1

On Thu, Feb 23, 2017 at 04:02:22PM +0100, Tomáš Golembiovský
wrote:
> v2:
> - 1/3: same as in previous version
> - 2/3: add check for libvirt version
> - 3/3: restore the disabled test
ACK series.

Will push in a minute once I've completed the tests.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/