Libguestfs - Aug 2016 - [PATCH] v2v: Allow libvirt >= 2.1.0 to be used for Xen and vCenter conversions.

Libvirt >= 2.1.0 now allows you to open files which have a "json:"
QEMU pseudo-URL as backingfile, whereas previously it would fail hard
in this case (RHBZ#1134878).

When virt-v2v performs conversions from Xen (over SSH) or vCenter
(over HTTPS) it uses these pseudo-URLs as backingfiles.  We had to
tell people to use LIBGUESTFS_BACKEND=direct to avoid libvirt in this
situation.

This commit narrows the check so it will now only print the error if
libvirt < 2.1.0 and LIBGUESTFS_BACKEND=direct is not set.  Also the
error is modified to tell users they can either upgrade libvirt or set
LIBGUESTFS_BACKEND=direct to work around the problem.

Note there is not an easy way apart from checking the version number
of libvirt to tell if the json pseudo-URL is supported.

As a side-effect, this commit also prints the libvirt version number
in debugging output when virt-v2v starts up, which is sometimes useful
information for narrowing down bugs (it is in fact already printed by
libguestfs, so this is duplicate information, but it's a bit easier to
find when it's at the top of the debug).

Thanks: Peter Krempa.
---
 v2v/domainxml-c.c                  | 38 ++++++++++++++++++++++++++++++++++++++
 v2v/domainxml.ml                   |  3 +++
 v2v/domainxml.mli                  |  4 ++++
 v2v/input_libvirt_other.ml         | 16 ++++++++++++----
 v2v/input_libvirt_other.mli        |  2 +-
 v2v/input_libvirt_vcenter_https.ml |  2 +-
 v2v/input_libvirt_xen_ssh.ml       |  2 +-
 v2v/v2v.ml                         | 12 ++++++++++++
 8 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/v2v/domainxml-c.c b/v2v/domainxml-c.c
index 3b00cad..eb6deab 100644
--- a/v2v/domainxml-c.c
+++ b/v2v/domainxml-c.c
@@ -487,6 +487,43 @@ v2v_domain_exists (value connv, value domnamev)
   CAMLreturn (Val_bool (domain_exists));
 }
 
+/* XXX This function is stuffed here for convenience (accessing
+ * libvirt), not because it belongs logically with the rest of the
+ * functions in this file.
+ */
+value
+v2v_libvirt_get_version (value unitv)
+{
+  CAMLparam1 (unitv);
+  CAMLlocal1 (rv);
+  int major, minor, release;
+  /* We have to assemble the error on the stack because a dynamic
+   * string couldn't be freed.
+   */
+  char errmsg[ERROR_MESSAGE_LEN];
+  unsigned long ver;
+  virErrorPtr err;
+
+  if (virGetVersion (&ver, NULL, NULL) == -1) {
+    err = virGetLastError ();
+    snprintf (errmsg, sizeof errmsg,
+              _("cannot get libvirt library version: %s"),
+              err->message);
+    caml_invalid_argument (errmsg);
+  }
+
+  major = ver / 1000000UL;
+  minor = ver / 1000UL % 1000UL;
+  release = ver % 1000UL;
+
+  rv = caml_alloc (3, 0);
+  Store_field (rv, 0, Val_int (major));
+  Store_field (rv, 1, Val_int (minor));
+  Store_field (rv, 2, Val_int (release));
+
+  CAMLreturn (rv);
+}
+
 #else /* !HAVE_LIBVIRT */
 
 #define NO_LIBVIRT(proto)                                               \
@@ -501,5 +538,6 @@ NO_LIBVIRT (value v2v_pool_dumpxml (value connv, value
poolv))
 NO_LIBVIRT (value v2v_vol_dumpxml (value connv, value poolnamev, value
volnamev))
 NO_LIBVIRT (value v2v_capabilities (value connv, value unitv))
 NO_LIBVIRT (value v2v_domain_exists (value connv, value domnamev))
+NO_LIBVIRT (value v2v_libvirt_get_version (value unitv))
 
 #endif /* !HAVE_LIBVIRT */
diff --git a/v2v/domainxml.ml b/v2v/domainxml.ml
index af053a5..b9d547d 100644
--- a/v2v/domainxml.ml
+++ b/v2v/domainxml.ml
@@ -26,3 +26,6 @@ external vol_dumpxml : ?conn:string -> string -> string
-> string = "v2v_vol_dum
 external capabilities : ?conn:string -> unit -> string =
"v2v_capabilities"
 
 external domain_exists : ?conn:string -> string -> bool =
"v2v_domain_exists"
+
+external libvirt_get_version : unit -> int * int * int
+  = "v2v_libvirt_get_version"
diff --git a/v2v/domainxml.mli b/v2v/domainxml.mli
index 17289bb..6d933e4 100644
--- a/v2v/domainxml.mli
+++ b/v2v/domainxml.mli
@@ -49,3 +49,7 @@ val domain_exists : ?conn:string -> string -> bool
     the libvirt XML domain [dom] exists.
     The optional [?conn] parameter is the libvirt connection URI.
     [dom] may be a guest name, but not a UUID. *)
+
+val libvirt_get_version : unit -> int * int * int
+(** [libvirt_get_version] returns the triple [(major, minor, release)]
+    version number of the libvirt library that we are linked against. *)
diff --git a/v2v/input_libvirt_other.ml b/v2v/input_libvirt_other.ml
index 6fd8d52..b2f3dc0 100644
--- a/v2v/input_libvirt_other.ml
+++ b/v2v/input_libvirt_other.ml
@@ -24,13 +24,21 @@ open Common_utils
 open Types
 open Utils
 
-(* Check the backend is not libvirt.  Works around a libvirt bug
- * (RHBZ#1134592).  This can be removed once the libvirt bug is fixed.
+(* Libvirt < 2.1.0 did not support the "json:" pseudo-URLs that
+ * we use as backingfiles, when accessing Xen over SSH or vCenter
+ * over HTTPS.  Check this and print a workaround.
+ *
+ * We can remove this when/if we ever require libvirt >= 2.1.0 as
+ * a minimum version.
+ *
+ * See also RHBZ#1134878.
  *)
-let error_if_libvirt_backend () +let
error_if_libvirt_does_not_support_json_backingfile ()    let libguestfs_backend
= (open_guestfs ())#get_backend () in
   if libguestfs_backend = "libvirt" then (
-    error (f_"because of libvirt bug
https://bugzilla.redhat.com/show_bug.cgi?id=1134592 you must set this
environment variable:\n\nexport LIBGUESTFS_BACKEND=direct\n\nand then rerun the
virt-v2v command.")
+    let major, minor, _ = Domainxml.libvirt_get_version () in
+    if major < 2 || (major = 2 && minor < 1) then
+      error (f_"because of libvirt bug https://bugzilla.redhat.com/1134878
you must EITHER upgrade to libvirt >= 2.1.0, OR set this environment
variable:\n\nexport LIBGUESTFS_BACKEND=direct\n\nand then rerun the virt-v2v
command.")
   )
 
 (* xen+ssh URLs use the SSH driver in CURL.  Currently this requires
diff --git a/v2v/input_libvirt_other.mli b/v2v/input_libvirt_other.mli
index 22cc21c..5b97f2f 100644
--- a/v2v/input_libvirt_other.mli
+++ b/v2v/input_libvirt_other.mli
@@ -18,7 +18,7 @@
 
 (** [-i libvirt] source. *)
 
-val error_if_libvirt_backend : unit -> unit
+val error_if_libvirt_does_not_support_json_backingfile : unit -> unit
 val error_if_no_ssh_agent : unit -> unit
 
 class virtual input_libvirt : string option -> string option -> string
-> object
diff --git a/v2v/input_libvirt_vcenter_https.ml
b/v2v/input_libvirt_vcenter_https.ml
index 137f3d3..bcedf3f 100644
--- a/v2v/input_libvirt_vcenter_https.ml
+++ b/v2v/input_libvirt_vcenter_https.ml
@@ -45,7 +45,7 @@ object
     debug "input_libvirt_vcenter_https: source: scheme %s server %s"
           scheme server;
 
-    error_if_libvirt_backend ();
+    error_if_libvirt_does_not_support_json_backingfile ();
 
     (* Get the libvirt XML.  This also checks (as a side-effect)
      * that the domain is not running.  (RHBZ#1138586)
diff --git a/v2v/input_libvirt_xen_ssh.ml b/v2v/input_libvirt_xen_ssh.ml
index 310b38b..f9adc58 100644
--- a/v2v/input_libvirt_xen_ssh.ml
+++ b/v2v/input_libvirt_xen_ssh.ml
@@ -38,7 +38,7 @@ object
     debug "input_libvirt_xen_ssh: source: scheme %s server %s"
           scheme server;
 
-    error_if_libvirt_backend ();
+    error_if_libvirt_does_not_support_json_backingfile ();
     error_if_no_ssh_agent ();
 
     (* Get the libvirt XML.  This also checks (as a side-effect)
diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index ccd369c..39054f7 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -44,6 +44,18 @@ let rec main ()          prog Guestfs_config.package_name
         Guestfs_config.package_version Guestfs_config.host_cpu;
 
+  (* Print the libvirt version if debugging.  Note that if
+   * we're configured --without-libvirt, then this will throw
+   * an exception, but some conversions should still be possible,
+   * hence the try block.
+   *)
+  if verbose () then (
+    try
+      let major, minor, release = Domainxml.libvirt_get_version () in
+      debug "libvirt version: %d.%d.%d" major minor release
+    with _ -> ()
+  );
+
   let source = open_source cmdline input in
   let source = amend_source cmdline source in
 
-- 
2.7.4

On Wednesday, 24 August 2016 15:01:23 CEST Richard W.M. Jones
wrote:
> Libvirt >= 2.1.0 now allows you to open files which have a
"json:"
> QEMU pseudo-URL as backingfile, whereas previously it would fail hard
> in this case (RHBZ#1134878).
> 
> When virt-v2v performs conversions from Xen (over SSH) or vCenter
> (over HTTPS) it uses these pseudo-URLs as backingfiles.  We had to
> tell people to use LIBGUESTFS_BACKEND=direct to avoid libvirt in this
> situation.
> 
> This commit narrows the check so it will now only print the error if
> libvirt < 2.1.0 and LIBGUESTFS_BACKEND=direct is not set.  Also the
> error is modified to tell users they can either upgrade libvirt or set
> LIBGUESTFS_BACKEND=direct to work around the problem.
> 
> Note there is not an easy way apart from checking the version number
> of libvirt to tell if the json pseudo-URL is supported.
> 
> As a side-effect, this commit also prints the libvirt version number
> in debugging output when virt-v2v starts up, which is sometimes useful
> information for narrowing down bugs (it is in fact already printed by
> libguestfs, so this is duplicate information, but it's a bit easier to
> find when it's at the top of the debug).
> 
> Thanks: Peter Krempa.
> ---
LGTM.

Thanks,
-- 
Pino Toscano