Libguestfs - Sep 2016 - [PATCH] v2v: -o glance: set all properties during creation (RHBZ#1374405)

The glance client v1.0.0 defaults to the Image v2 API: this means that
an image can be referenced only by its UUID, and not anymore by the name
as well (which does not need to be unique).  This caused our glance
invocation to set the properties for the newly created image to fail,
since we are using the name as identifier.

Instead of first creating the image and then setting all the properties
for it, set all the properties when creating the image: other than being
simplier, it also avoid parsing the output of the 'glance image-create'
command for the UUID ('id') of the image.
---
 v2v/output_glance.ml | 28 +++++++---------------------
 1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml
index d2660a7..399b45c 100644
--- a/v2v/output_glance.ml
+++ b/v2v/output_glance.ml
@@ -73,12 +73,6 @@ object
           if i == 0 then source.s_name
           else sprintf "%s-disk%d" source.s_name (i+1) in
 
-        let cmd = [ "glance"; "image-create";
"--name"; name;
-                    "--disk-format=" ^ target_format;
-                    "--container-format=bare"; "--file";
target_file ] in
-        if run_command cmd <> 0 then
-          error (f_"glance: image upload to glance failed, see earlier
errors");
-
         (* Set the properties (ie. metadata). *)
         let min_ram = source.s_memory /^ 1024L /^ 1024L in
         let properties = [
@@ -114,26 +108,18 @@ object
           | x, 0 -> ("os_version", string_of_int x) :: properties
           | x, y -> ("os_version", sprintf "%d.%d" x y)
:: properties in
 
-        (* Glance doesn't appear to check the properties. *)
-        let cmd = [ "glance"; "image-update";
"--min-ram";
-                    Int64.to_string min_ram ] @
+        let cmd = [ "glance"; "image-create";
"--name"; name;
+                    "--disk-format=" ^ target_format;
+                    "--container-format=bare"; "--file";
target_file;
+                    "--min-ram"; Int64.to_string min_ram ] @
                   (List.flatten
                     (List.map (
                        fun (k, v) ->
                          [ "--property"; sprintf "%s=%s" k
v ]
                     ) properties
-                  )) @
-                  [ name ] in
-        if run_command cmd <> 0 then (
-          warning (f_"glance: failed to set image properties
(ignored)");
-          (* Dump out the image properties so the user can set them. *)
-          eprintf "Image properties:\n";
-          eprintf "  --min-ram %Ld\n" min_ram;
-          List.iter (
-	    fun (k, v) ->
-	      eprintf "  --property %s=%s" (quote k) (quote v)
-          ) properties
-        )
+                  )) in
+        if run_command cmd <> 0 then
+          error (f_"glance: image upload to glance failed, see earlier
errors");
       ) targets
 end
 
-- 
2.7.4

On Tue, Sep 13, 2016 at 03:40:33PM +0200, Pino Toscano
wrote:
> The glance client v1.0.0 defaults to the Image v2 API: this means that
> an image can be referenced only by its UUID, and not anymore by the name
> as well (which does not need to be unique).  This caused our glance
> invocation to set the properties for the newly created image to fail,
> since we are using the name as identifier.
> 
> Instead of first creating the image and then setting all the properties
> for it, set all the properties when creating the image: other than being
> simplier, it also avoid parsing the output of the 'glance
image-create'
^ simpler
> command for the UUID ('id') of the image.
> ---
>  v2v/output_glance.ml | 28 +++++++---------------------
>  1 file changed, 7 insertions(+), 21 deletions(-)
> 
> diff --git a/v2v/output_glance.ml b/v2v/output_glance.ml
> index d2660a7..399b45c 100644
> --- a/v2v/output_glance.ml
> +++ b/v2v/output_glance.ml
> @@ -73,12 +73,6 @@ object
>            if i == 0 then source.s_name
>            else sprintf "%s-disk%d" source.s_name (i+1) in
>  
> -        let cmd = [ "glance"; "image-create";
"--name"; name;
> -                    "--disk-format=" ^ target_format;
> -                    "--container-format=bare";
"--file"; target_file ] in
> -        if run_command cmd <> 0 then
> -          error (f_"glance: image upload to glance failed, see
earlier errors");
> -
>          (* Set the properties (ie. metadata). *)
>          let min_ram = source.s_memory /^ 1024L /^ 1024L in
>          let properties = [
> @@ -114,26 +108,18 @@ object
>            | x, 0 -> ("os_version", string_of_int x) ::
properties
>            | x, y -> ("os_version", sprintf "%d.%d"
x y) :: properties in
>  
> -        (* Glance doesn't appear to check the properties. *)
> -        let cmd = [ "glance"; "image-update";
"--min-ram";
> -                    Int64.to_string min_ram ] @
> +        let cmd = [ "glance"; "image-create";
"--name"; name;
> +                    "--disk-format=" ^ target_format;
> +                    "--container-format=bare";
"--file"; target_file;
> +                    "--min-ram"; Int64.to_string min_ram ] @
>                    (List.flatten
>                      (List.map (
>                         fun (k, v) ->
>                           [ "--property"; sprintf
"%s=%s" k v ]
>                      ) properties
> -                  )) @
> -                  [ name ] in
> -        if run_command cmd <> 0 then (
> -          warning (f_"glance: failed to set image properties
(ignored)");
> -          (* Dump out the image properties so the user can set them. *)
> -          eprintf "Image properties:\n";
> -          eprintf "  --min-ram %Ld\n" min_ram;
> -          List.iter (
> -	    fun (k, v) ->
> -	      eprintf "  --property %s=%s" (quote k) (quote v)
> -          ) properties
> -        )
> +                  )) in
> +        if run_command cmd <> 0 then
> +          error (f_"glance: image upload to glance failed, see
earlier errors");
>        ) targets
>  end
It does indeed seem simpler, so ACK.

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/