Libguestfs - Jan 2019 - Re: [PATCH] v2v: -o rhv-upload: Allow configure commands to set the Python version.

No functional change, but it does allow downstream distributions to
adjust the Python interpreter and nbdkit Python plugin used by
virt-v2v -o rhv-upload mode:

./configure --with-virt-v2v-python-interpreter=...
./configure --with-virt-v2v-nbdkit-python-plugin=...

If you don't set the configure parameters then this change makes no
difference.
---
 .gitignore               |  1 +
 configure.ac             |  5 +++++
 m4/guestfs-v2v.m4        | 38 ++++++++++++++++++++++++++++++++++++++
 v2v/Makefile.am          |  4 ++++
 v2v/config.ml.in         | 21 +++++++++++++++++++++
 v2v/config.mli           | 37 +++++++++++++++++++++++++++++++++++++
 v2v/output_rhv_upload.ml | 18 +++++++++++++++---
 v2v/python_script.ml     | 15 +++++----------
 v2v/python_script.mli    | 14 +++++---------
 9 files changed, 131 insertions(+), 22 deletions(-)

diff --git a/.gitignore b/.gitignore
index fe5aa6d70..637bf7765 100644
--- a/.gitignore
+++ b/.gitignore
@@ -678,6 +678,7 @@ Makefile.in
 /utils/qemu-boot/qemu-boot
 /utils/qemu-speed-test/qemu-speed-test
 /v2v/.depend
+/v2v/config.ml
 /v2v/oUnit-*
 /v2v/output_rhv_upload_*_source.ml
 /v2v/real-*.d/
diff --git a/configure.ac b/configure.ac
index 143435b36..a94f3abab 100644
--- a/configure.ac
+++ b/configure.ac
@@ -162,6 +162,10 @@ m4_include([m4/guestfs-golang.m4])
 HEADING([Checking for GObject Introspection])
 m4_include([m4/guestfs-gobject.m4])
 
+dnl virt-v2v, virt-p2v.
+HEADING([Checking the virt-v2v and virt-p2v dependencies])
+m4_include([m4/guestfs-v2v.m4])
+
 dnl Bash completion.
 HEADING([Checking for bash completion])
 m4_include([m4/guestfs-bash-completion.m4])
@@ -365,6 +369,7 @@ AC_CONFIG_FILES([Makefile
                  utils/qemu-boot/Makefile
                  utils/qemu-speed-test/Makefile
                  v2v/Makefile
+                 v2v/config.ml
                  v2v/test-harness/Makefile
                  v2v/test-harness/META
                  website/index.html])
diff --git a/m4/guestfs-v2v.m4 b/m4/guestfs-v2v.m4
new file mode 100644
index 000000000..3b85d6d54
--- /dev/null
+++ b/m4/guestfs-v2v.m4
@@ -0,0 +1,38 @@
+# libguestfs
+# Copyright (C) 2009-2019 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+dnl Virt-v2v and virt-p2v.
+
+dnl Python interpreter and nbdkit python plugin.
+dnl Note the name "python3" is defined by PEP 394.
+AC_MSG_CHECKING([for the python interpreter used by virt-v2v])
+AC_ARG_WITH([virt-v2v-python-interpreter],
+   
[AS_HELP_STRING([--with-virt-v2v-python-interpreter="python3|..."],
+        [set Python interpreter used by virt-v2v
@<:@default=python3@:>@])],
+    [VIRT_V2V_PYTHON_INTERPRETER="$withval"],
+    [VIRT_V2V_PYTHON_INTERPRETER=python3])
+AC_MSG_RESULT([$VIRT_V2V_PYTHON_INTERPRETER])
+AC_SUBST([VIRT_V2V_PYTHON_INTERPRETER])
+
+AC_MSG_CHECKING([for the nbdkit python plugin name])
+AC_ARG_WITH([virt-v2v-nbdkit-python-plugin],
+   
[AS_HELP_STRING([--with-virt-v2v-nbdkit-python-plugin="python|..."],
+        [set nbdkit python plugin name used by virt-v2v
@<:@default=python@:>@])],
+    [VIRT_V2V_NBDKIT_PYTHON_PLUGIN="$withval"],
+    [VIRT_V2V_NBDKIT_PYTHON_PLUGIN=python])
+AC_MSG_RESULT([$VIRT_V2V_NBDKIT_PYTHON_PLUGIN])
+AC_SUBST([VIRT_V2V_NBDKIT_PYTHON_PLUGIN])
diff --git a/v2v/Makefile.am b/v2v/Makefile.am
index a156524ae..2312812fb 100644
--- a/v2v/Makefile.am
+++ b/v2v/Makefile.am
@@ -23,6 +23,7 @@ generator_built = \
 
 BUILT_SOURCES = \
 	$(generator_built) \
+	config.ml \
 	output_rhv_upload_createvm_source.ml \
 	output_rhv_upload_plugin_source.ml \
 	output_rhv_upload_precheck_source.ml
@@ -48,6 +49,7 @@ EXTRA_DIST = \
 SOURCES_MLI = \
 	changeuid.mli \
 	cmdline.mli \
+	config.mli \
 	convert_linux.mli \
 	convert_windows.mli \
 	create_libvirt_xml.mli \
@@ -100,6 +102,7 @@ SOURCES_MLI = \
 	windows_virtio.mli
 
 SOURCES_ML = \
+	config.ml \
 	types.ml \
 	uefi.ml \
 	utils.ml \
@@ -697,6 +700,7 @@ v2v_unit_tests_LINK = \
 .depend: \
 	$(srcdir)/*.mli \
 	$(srcdir)/*.ml \
+	config.ml \
 	output_rhv_upload_*_source.ml
 	$(top_builddir)/ocaml-dep.sh $^
 -include .depend
diff --git a/v2v/config.ml.in b/v2v/config.ml.in
new file mode 100644
index 000000000..f24cd1629
--- /dev/null
+++ b/v2v/config.ml.in
@@ -0,0 +1,21 @@
+(* virt-v2v
+ * Copyright (C) 2019 Red Hat Inc.
+ * @configure_input@
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *)
+
+let virt_v2v_python_interpreter = "@VIRT_V2V_PYTHON_INTERPRETER@"
+let virt_v2v_nbdkit_python_plugin = "@VIRT_V2V_NBDKIT_PYTHON_PLUGIN@"
diff --git a/v2v/config.mli b/v2v/config.mli
new file mode 100644
index 000000000..327a68016
--- /dev/null
+++ b/v2v/config.mli
@@ -0,0 +1,37 @@
+(* virt-v2v
+ * Copyright (C) 2019 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ *)
+
+val virt_v2v_python_interpreter : string
+(** Return the name of the Python interpreter used to run scripts by
+    [virt-v2v -o rhv-upload].
+
+    This must normally be a Python 3 interpreter.  However some
+    downstream distributions replace the Python 3 scripts with Python
+    2 scripts and use the following configure option to adjust this
+    interpreter accordingly:
+
+    [./configure --with-virt-v2v-python-interpreter=...] *)
+
+val virt_v2v_nbdkit_python_plugin : string
+(** Return the name of the nbdkit python plugin used by
+    [virt-v2v -o rhv-upload].
+
+    As above this must also be the Python 3 version of the plugin,
+    unless you change it.  The configure command to change this is:
+
+    [./configure --with-virt-v2v-nbdkit-python-plugin=...] *)
diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
index c309e31e3..e0416a842 100644
--- a/v2v/output_rhv_upload.ml
+++ b/v2v/output_rhv_upload.ml
@@ -78,7 +78,8 @@ let parse_output_options options  
   { rhv_cafile; rhv_cluster; rhv_direct; rhv_verifypeer }
 
-let nbdkit_python_plugin = "python"
+let python_intepreter = Config.virt_v2v_python_interpreter
+let nbdkit_python_plugin = Config.virt_v2v_nbdkit_python_plugin
 let pidfile_timeout = 30
 let finalization_timeout = 5*60
 
@@ -97,21 +98,32 @@ class output_rhv_upload output_alloc output_conn
   (* Create Python scripts for precheck, plugin and create VM. *)
   let precheck_script      Python_script.create
~name:"rhv-upload-precheck.py"
+                         ~python:python_intepreter
                          Output_rhv_upload_precheck_source.code in
   let plugin_script      Python_script.create
~name:"rhv-upload-plugin.py"
+                         ~python:python_intepreter
                          Output_rhv_upload_plugin_source.code in
   let createvm_script      Python_script.create
~name:"rhv-upload-createvm.py"
+                         ~python:python_intepreter
                          Output_rhv_upload_createvm_source.code in
 
   (* Is SELinux enabled and enforcing on the host? *)
   let have_selinux      0 = Sys.command "getenforce 2>/dev/null | grep
-isq Enforcing" in
 
+  (* Check Python interpreter found. *)
+  let error_unless_python_interpreter_found () +    try ignore (which
python_intepreter)
+    with Executable_not_found _ ->
+      error (f_"no python binary called ‘%s’ can be found on the
$PATH")
+            python_intepreter
+  in
+
   (* Check that the 'ovirtsdk4' Python module is available. *)
   let error_unless_ovirtsdk4_module_available () -    let res = run_command [
Python_script.python; "-c"; "import ovirtsdk4" ] in
+    let res = run_command [ python_intepreter; "-c"; "import
ovirtsdk4" ] in
     if res <> 0 then
       error (f_"the Python module ‘ovirtsdk4’ could not be loaded, is it
installed?  See previous messages for problems.")
   in
@@ -225,7 +237,7 @@ object
   inherit output
 
   method precheck () -    Python_script.error_unless_python_interpreter_found
();
+    error_unless_python_interpreter_found ();
     error_unless_ovirtsdk4_module_available ();
     error_unless_nbdkit_working ();
     error_unless_nbdkit_python_plugin_working ();
diff --git a/v2v/python_script.ml b/v2v/python_script.ml
index 3159373a1..9b7525b95 100644
--- a/v2v/python_script.ml
+++ b/v2v/python_script.ml
@@ -24,14 +24,15 @@ open Unix_utils
 
 open Common_gettext.Gettext
 
-let python = "python3"          (* Defined by PEP 394 *)
+let python = Config.virt_v2v_python_interpreter
 
 type script = {
   tmpdir : string;              (* Temporary directory. *)
+  python : string;              (* Python interpreter. *)
   path : string;                (* Path to script. *)
 }
 
-let create ?(name = "script.py") code +let create ?(name =
"script.py") ~python code    let tmpdir      let base_dir =
(open_guestfs ())#get_cachedir () in
     let t = Mkdtemp.temp_dir ~base_dir "v2v." in
@@ -39,10 +40,10 @@ let create ?(name = "script.py") code      t in
   let path = tmpdir // name in
   with_open_out path (fun chan -> output_string chan code);
-  { tmpdir; path }
+  { tmpdir; python; path }
 
 let run_command ?echo_cmd ?stdout_fd ?stderr_fd
-                { tmpdir; path } params args +                { tmpdir; python;
path } params args    let param_file = tmpdir // sprintf
"params%d.json" (unique ()) in
   with_open_out
     param_file
@@ -51,9 +52,3 @@ let run_command ?echo_cmd ?stdout_fd ?stderr_fd
                           (python :: path :: param_file :: args)
 
 let path { path } = path
-
-let error_unless_python_interpreter_found () -  try ignore (which python)
-  with Executable_not_found _ ->
-    error (f_"no python binary called ‘%s’ can be found on the
$PATH")
-          python
diff --git a/v2v/python_script.mli b/v2v/python_script.mli
index c008eec1a..580d16381 100644
--- a/v2v/python_script.mli
+++ b/v2v/python_script.mli
@@ -20,12 +20,15 @@
 
 type script
 
-val create : ?name:string -> string -> script
+val create : ?name:string -> python:string -> string -> script
 (** Create a Python script object.
 
     The optional parameter [?name] is a hint for the name of the script.
 
-    The parameter is the Python code.  Usually this is
+    The named parameter is the Python interpreter to use.  Usually
+    you would use {!Config.virt_v2v_python_interpreter}.
+
+    The unnamed parameter is the Python code.  Usually this is
     [Some_source.code] where [some_source.ml] is generated from
     the Python file by [v2v/embed.sh] (see also [v2v/Makefile.am]). *)
 
@@ -54,10 +57,3 @@ val path : script -> string
     try using/storing it beyond the lifetime of the program.
 
     This is used only where {!run_command} is not suitable. *)
-
-val python : string
-(** Return the name of the Python interpreter. *)
-
-val error_unless_python_interpreter_found : unit -> unit
-(** Check if the Python interpreter can be found on the path, and
-    call [error] (which is fatal) if not. *)
-- 
2.19.2

On Tuesday, 8 January 2019 13:29:47 CET Richard W.M. Jones
wrote:
> No functional change, but it does allow downstream distributions to
> adjust the Python interpreter and nbdkit Python plugin used by
> virt-v2v -o rhv-upload mode:
> 
> ./configure --with-virt-v2v-python-interpreter=...
What is the use case for this? The documentation for the variable in
config.mli says:
> +val virt_v2v_python_interpreter : string
> +(** Return the name of the Python interpreter used to run scripts by
> +    [virt-v2v -o rhv-upload].
> +
> +    This must normally be a Python 3 interpreter.  However some
> +    downstream distributions replace the Python 3 scripts with Python
> +    2 scripts and use the following configure option to adjust this
> +    interpreter accordingly:
> +
> +    [./configure --with-virt-v2v-python-interpreter=...] *)
To support Python 2 way more changes are needed though, other than the
interpreter, so patching one line more in python_script.ml is not a big
deal in that case.
> ./configure --with-virt-v2v-nbdkit-python-plugin=...
This would be OKish, although needed because of downstream changes in
Fedora (where the Python plugin of nbdkit is built also as Python 3,
and this version is renamed to "python3").

-- 
Pino Toscano

On Fri, Jan 11, 2019 at 05:27:59PM +0100, Pino Toscano
wrote:
> On Tuesday, 8 January 2019 13:29:47 CET Richard W.M. Jones wrote:
> > No functional change, but it does allow downstream distributions to
> > adjust the Python interpreter and nbdkit Python plugin used by
> > virt-v2v -o rhv-upload mode:
> > 
> > ./configure --with-virt-v2v-python-interpreter=...
> 
> What is the use case for this? The documentation for the variable in
> config.mli says:
> 
> > +val virt_v2v_python_interpreter : string
> > +(** Return the name of the Python interpreter used to run scripts by
> > +    [virt-v2v -o rhv-upload].
> > +
> > +    This must normally be a Python 3 interpreter.  However some
> > +    downstream distributions replace the Python 3 scripts with Python
> > +    2 scripts and use the following configure option to adjust this
> > +    interpreter accordingly:
> > +
> > +    [./configure --with-virt-v2v-python-interpreter=...] *)
> 
> To support Python 2 way more changes are needed though, other than the
> interpreter, so patching one line more in python_script.ml is not a big
> deal in that case.
I expect there will be some distros where the Python 3 interpreter is
called "python" (or something else odd).  In any case you are right
that this doesn't by any means cover all the changes required for
Python 2.
> > ./configure --with-virt-v2v-nbdkit-python-plugin=...
> 
> This would be OKish, although needed because of downstream changes in
> Fedora (where the Python plugin of nbdkit is built also as Python 3,
> and this version is renamed to "python3").
I wish one day we will be able to get rid of this hack :-)

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/