Libguestfs - Oct 2017 - Re: [PATCH] lib: Pick up qemu-img path at build time.

The main purpose of this change is two-fold:

(1) Ensure that we run the same version of qemu-img that we are
built against.

(2) Allow the qemu-img path to be overridden at build time in case
it's on a nonstandard path or (like RHV) has a nonstandard name.
---
 docs/guestfs-building.pod | 6 ++++++
 lib/command.c             | 4 ++--
 lib/create.c              | 4 ++--
 lib/info.c                | 2 +-
 m4/guestfs-qemu.m4        | 4 ++++
 5 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/docs/guestfs-building.pod b/docs/guestfs-building.pod
index d85483a0b..94df49158 100644
--- a/docs/guestfs-building.pod
+++ b/docs/guestfs-building.pod
@@ -716,6 +716,12 @@ binary to find the version of Python, the location of
Python libraries
 and so on.  See
 L</BUILDING PYTHON 2 AND PYTHON 3 BINDINGS> below.
 
+=item B<QEMU_IMG>
+
+This environment variable may be set to point to the full path of the
+L<qemu-img(1)> program, in case it is on a nonstandard path or has a
+nonstandard name.
+
 =item B<SUPERMIN>
 
 This environment variable can be set to choose an alternative
diff --git a/lib/command.c b/lib/command.c
index bfec76f19..3d8bc7dbf 100644
--- a/lib/command.c
+++ b/lib/command.c
@@ -35,7 +35,7 @@
  *
  * I<Either> add arguments:
  *
- *  guestfs_int_cmd_add_arg (cmd, "qemu-img");
+ *  guestfs_int_cmd_add_arg (cmd, QEMU_IMG);
  *  guestfs_int_cmd_add_arg (cmd, "info");
  *  guestfs_int_cmd_add_arg (cmd, filename);
  *
@@ -48,7 +48,7 @@
  * shell commands, with the added safety of allowing args to be quoted
  * properly).
  *
- *  guestfs_int_cmd_add_string_unquoted (cmd, "qemu-img info ");
+ *  guestfs_int_cmd_add_string_unquoted (cmd, "file info ");
  *  guestfs_int_cmd_add_string_quoted (cmd, filename);
  *
  * =item 4.
diff --git a/lib/create.c b/lib/create.c
index fff5cf332..b4865172c 100644
--- a/lib/create.c
+++ b/lib/create.c
@@ -314,7 +314,7 @@ disk_create_qcow2 (guestfs_h *g, const char *orig_filename,
int64_t size,
   }
 
   /* Assemble the qemu-img command line. */
-  guestfs_int_cmd_add_arg (cmd, "qemu-img");
+  guestfs_int_cmd_add_arg (cmd, QEMU_IMG);
   guestfs_int_cmd_add_arg (cmd, "create");
   guestfs_int_cmd_add_arg (cmd, "-f");
   guestfs_int_cmd_add_arg (cmd, "qcow2");
@@ -347,7 +347,7 @@ disk_create_qcow2 (guestfs_h *g, const char *orig_filename,
int64_t size,
 
   r = guestfs_int_cmd_run (cmd);
   if (!WIFEXITED (r) || WEXITSTATUS (r) != 0) {
-    guestfs_int_external_command_failed (g, r, "qemu-img",
orig_filename);
+    guestfs_int_external_command_failed (g, r, QEMU_IMG, orig_filename);
     return -1;
   }
 
diff --git a/lib/info.c b/lib/info.c
index f7378adfd..4464df994 100644
--- a/lib/info.c
+++ b/lib/info.c
@@ -191,7 +191,7 @@ get_json_output (guestfs_h *g, const char *filename)
   snprintf (fdpath, sizeof fdpath, "/dev/fd/%d", fd);
   guestfs_int_cmd_clear_close_files (cmd);
 
-  guestfs_int_cmd_add_arg (cmd, "qemu-img");
+  guestfs_int_cmd_add_arg (cmd, QEMU_IMG);
   guestfs_int_cmd_add_arg (cmd, "info");
   guestfs_int_cmd_add_arg (cmd, "--output");
   guestfs_int_cmd_add_arg (cmd, "json");
diff --git a/m4/guestfs-qemu.m4 b/m4/guestfs-qemu.m4
index 350bb980d..6ea7ef3d7 100644
--- a/m4/guestfs-qemu.m4
+++ b/m4/guestfs-qemu.m4
@@ -100,3 +100,7 @@ working.
     esac
     AC_DEFINE_UNQUOTED([QEMU],["$QEMU"],[Location of qemu binary.])
 ])
+
+dnl qemu-img (required).
+AC_PATH_PROGS([QEMU_IMG],[qemu-img],[no])
+AC_DEFINE_UNQUOTED([QEMU_IMG],["$QEMU_IMG"],[Path to qemu-img
program])
-- 
2.13.2

On Friday, 6 October 2017 15:59:42 CEST Richard W.M. Jones
wrote:
> The main purpose of this change is two-fold:
> 
> (1) Ensure that we run the same version of qemu-img that we are
> built against.
> 
> (2) Allow the qemu-img path to be overridden at build time in case
> it's on a nonstandard path or (like RHV) has a nonstandard name.
> ---
As I mentioned in other series, I am not a fan of hardcoding at build
time paths used at runtime.  This patch has different issues:
a) the hardcoding itself (which IMHO is a step backward)
b) what the patch does just cover a very minimal set of all the
   qemu-img invocations; all the tools rely on $PATH
c) it is not overridable at all: since qemu can be changed in different
   ways (API entry, envvar), it is not logic that qemu-img (built from
   the same sources of qemu) cannot be changed at all; this situation
   e.g. makes it not possible to use a custom qemu build (that says
   enables some feature) in libguestfs
d) the point (2) above implies supporting RHV would mean rebuilding
   libguestfs just for that, which makes testing harder than needed

qemu is the "special flower" in this situation, since it can be
outside
of $PATH (hi RHEL), with a custom name, etc.  qemu-img does not have any
of the qemu peculiarities.

-- 
Pino Toscano

On Fri, Oct 06, 2017 at 04:50:44PM +0200, Pino Toscano
wrote:
> On Friday, 6 October 2017 15:59:42 CEST Richard W.M. Jones wrote:
> > The main purpose of this change is two-fold:
> > 
> > (1) Ensure that we run the same version of qemu-img that we are
> > built against.
> > 
> > (2) Allow the qemu-img path to be overridden at build time in case
> > it's on a nonstandard path or (like RHV) has a nonstandard name.
> > ---
> 
> As I mentioned in other series, I am not a fan of hardcoding at build
> time paths used at runtime.  This patch has different issues:
> a) the hardcoding itself (which IMHO is a step backward)
> b) what the patch does just cover a very minimal set of all the
>    qemu-img invocations; all the tools rely on $PATH
> c) it is not overridable at all: since qemu can be changed in different
>    ways (API entry, envvar), it is not logic that qemu-img (built from
>    the same sources of qemu) cannot be changed at all; this situation
>    e.g. makes it not possible to use a custom qemu build (that says
>    enables some feature) in libguestfs
> d) the point (2) above implies supporting RHV would mean rebuilding
>    libguestfs just for that, which makes testing harder than needed
> 
> qemu is the "special flower" in this situation, since it can be
outside
> of $PATH (hi RHEL), with a custom name, etc.  qemu-img does not have any
> of the qemu peculiarities.
This is all true, but unless we make qemu-img configurable at runtime
(which is extra complexity), I'm not sure what to do about it.

FWIW in RHEL qemu-img and qemu-img-rhev both ship /usr/bin/qemu-img.
(I previously thought that the binaries had different names, did that
change recently or was a I just mis-remembering??)

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