Libguestfs - Nov 2018 - [PATCH] v2v: -o openstack: Check openstack binary exists before running it.

Improves the error message when openstack is not installed:

$ virt-v2v -i disk fedora-28.img -o openstack
virt-v2v: error: no binary called ‘openstack’ was found on the $PATH.
We use this program to communicate with OpenStack so it must be installed
to use this output mode.
---
 v2v/output_openstack.ml | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/v2v/output_openstack.ml b/v2v/output_openstack.ml
index b5bbc9195..238ec99a9 100644
--- a/v2v/output_openstack.ml
+++ b/v2v/output_openstack.ml
@@ -28,6 +28,9 @@ open Common_gettext.Gettext
 open Types
 open Utils
 
+(* Name of the openstack CLI program (on $PATH). *)
+let openstack_binary = "openstack"
+
 (* Timeout waiting for new Cinder volumes to move to "available"
state.
  * We assume this could be quite a long time on backends which want
  * to preallocate the storage.
@@ -172,13 +175,20 @@ class output_openstack output_conn output_password
output_storage
        object_get_string "uuid" json
   ) in
 
+  let error_unless_openstack_command_exists () +    try ignore (which
openstack_binary)
+    with Executable_not_found _ ->
+      error (f_"no binary called ‘%s’ was found on the $PATH.  We use this
program to communicate with OpenStack so it must be installed to use this output
mode.")
+         openstack_binary
+  in
+
   (* We use this convenient wrapper around [Tools_utils.run_command]
    * for two reasons: (1) Because we want to run openstack with
    * extra_args.  (2) OpenStack commands are noisy so we want to
    * direct stdout to /dev/null unless we're in verbose mode.
    *)
   let run_openstack_command args -    let cmd = [ "openstack" ] @
extra_args @ args in
+    let cmd = [ openstack_binary ] @ extra_args @ args in
     let stdout_fd        if verbose () then None
       else Some (openfile "/dev/null" [O_WRONLY] 0) in
@@ -191,7 +201,7 @@ class output_openstack output_conn output_password
output_storage
    * '-f json' to the args yourself.
    *)
   let run_openstack_command_capture_json args -    let cmd = [
"openstack" ] @ extra_args @ args in
+    let cmd = [ openstack_binary ] @ extra_args @ args in
 
     let json, chan = Filename.open_temp_file "v2vopenstack"
".json" in
     unlink_on_exit json;
@@ -366,6 +376,9 @@ object
   inherit output
 
   method precheck () +    (* Check the openstack command exists. *)
+    error_unless_openstack_command_exists ();
+
     (* Run the openstack command simply to check we can connect
      * with the provided authentication parameters/environment
      * variables.  Issuing a token should have only a tiny
-- 
2.19.0.rc0

On Tuesday, 20 November 2018 10:16:28 CET Richard W.M. Jones
wrote:
> Improves the error message when openstack is not installed:
> 
> $ virt-v2v -i disk fedora-28.img -o openstack
> virt-v2v: error: no binary called ‘openstack’ was found on the $PATH.
> We use this program to communicate with OpenStack so it must be installed
> to use this output mode.
> ---
Generally OK, just one minor note below.
> +  let error_unless_openstack_command_exists () > +    try ignore (which
openstack_binary)
> +    with Executable_not_found _ ->
> +      error (f_"no binary called ‘%s’ was found on the $PATH.  We use
this program to communicate with OpenStack so it must be installed to use this
output mode.")
> +         openstack_binary
IMHO the error string is a bit too much verbose -- what about something
like:
"the ‘%s’ program is not available.  It is needed to communicate with
 OpenStack"

Also, I think it would be good to mention the requirement of this
command line client in virt-v2v-output-openstack(1), and "glance" for
-o glance too.

-- 
Pino Toscano