Libguestfs - Sep 2019 - Re: [PATCH] v2v: -o rhv-upload: add -oo rhv-disk-uuid option

This way it is possible to override the UUIDs of the uploaded disks,
instead of letting RHV generate them.

This can be useful to force certain UUIDs, and to specify the disks in
--no-copy mode (which now can be used).
---
 v2v/output_rhv_upload.ml    | 43 ++++++++++++++++++++++++++++++++-----
 v2v/rhv-upload-plugin.py    |  2 ++
 v2v/virt-v2v-output-rhv.pod | 23 ++++++++++++++++++++
 3 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/v2v/output_rhv_upload.ml b/v2v/output_rhv_upload.ml
index 40902c371..eec9c5c79 100644
--- a/v2v/output_rhv_upload.ml
+++ b/v2v/output_rhv_upload.ml
@@ -32,6 +32,7 @@ type rhv_options = {
   rhv_cluster : string option;
   rhv_direct : bool;
   rhv_verifypeer : bool;
+  rhv_disk_uuids : string list option;
 }
 
 let print_output_options () @@ -41,6 +42,11 @@ let print_output_options ()   
-oo rhv-cluster=CLUSTERNAME     Set RHV cluster name.
   -oo rhv-direct[=true|false]     Use direct transfer mode (default: false).
   -oo rhv-verifypeer[=true|false] Verify server identity (default: false).
+
+You can override the UUIDs of the disks, instead of using autogenerated UUIDs
+after their uploads (if you do, you must supply one for each disk):
+
+  -oo rhv-disk-uuid=UUID          Disk UUID
 ")
 
 let parse_output_options options @@ -48,6 +54,7 @@ let parse_output_options
options    let rhv_cluster = ref None in
   let rhv_direct = ref false in
   let rhv_verifypeer = ref false in
+  let rhv_disk_uuids = ref None in
 
   List.iter (
     function
@@ -63,6 +70,8 @@ let parse_output_options options      |
"rhv-direct", v -> rhv_direct := bool_of_string v
     | "rhv-verifypeer", "" -> rhv_verifypeer := true
     | "rhv-verifypeer", v -> rhv_verifypeer := bool_of_string v
+    | "rhv-disk-uuid", v ->
+       rhv_disk_uuids := Some (v :: (Option.default [] !rhv_disk_uuids))
     | k, _ ->
        error (f_"-o rhv-upload: unknown output option ‘-oo %s’") k
   ) options;
@@ -75,8 +84,9 @@ let parse_output_options options    let rhv_cluster =
!rhv_cluster in
   let rhv_direct = !rhv_direct in
   let rhv_verifypeer = !rhv_verifypeer in
+  let rhv_disk_uuids = Option.map List.rev !rhv_disk_uuids in
 
-  { rhv_cafile; rhv_cluster; rhv_direct; rhv_verifypeer }
+  { rhv_cafile; rhv_cluster; rhv_direct; rhv_verifypeer; rhv_disk_uuids }
 
 let nbdkit_python_plugin = Config.virt_v2v_nbdkit_python_plugin
 let pidfile_timeout = 30
@@ -283,6 +293,16 @@ object
   method install_rhev_apt = true
 
   method prepare_targets source overlays _ _ _ _ +    let uuids +      match
rhv_options.rhv_disk_uuids with
+      | None ->
+        List.map (fun _ -> None) overlays
+      | Some uuids ->
+        if List.length uuids <> List.length overlays then
+          error (f_"the number of ‘-oo rhv-disk-uuid’ parameters passed on
the command line has to match the number of guest disk images (for this guest:
%d)")
+            (List.length overlays);
+        List.map (fun uuid -> Some uuid) uuids in
+
     let output_name = source.s_name in
     let json_params        ("output_name", JSON.String output_name)
:: json_params in
@@ -306,7 +326,7 @@ object
      * target URI to point to the NBD socket.
      *)
     List.map (
-      fun (target_format, ov) ->
+      fun ((target_format, ov), uuid) ->
         let id = ov.ov_source.s_disk_id in
         let disk_name = sprintf "%s-%03d" output_name id in
         let json_params @@ -332,6 +352,12 @@ object
         let json_params            ("diskid_file", JSON.String
diskid_file) :: json_params in
 
+        let json_params +          match uuid with
+          | None -> json_params
+          | Some uuid ->
+            ("rhv_disk_uuid", JSON.String uuid) :: json_params in
+
         (* Write the JSON parameters to a file. *)
         let json_param_file = tmpdir // sprintf "params%d.json" id in
         with_open_out
@@ -401,7 +427,7 @@ If the messages above are not sufficient to diagnose the
problem then add the
           "file.export", JSON.String "/";
         ] in
         TargetURI ("json:" ^ JSON.string_of_doc json_params)
-    ) overlays
+    ) (List.combine overlays uuids)
 
   method disk_copied t i nr_disks      (* Get the UUID of the disk image.  This
file is written
@@ -417,7 +443,14 @@ If the messages above are not sufficient to diagnose the
problem then add the
     disks_uuids <- disks_uuids @ [diskid];
 
   method create_metadata source targets _ guestcaps inspect target_firmware -  
assert (List.length disks_uuids = List.length targets);
+    let image_uuids +      match rhv_options.rhv_disk_uuids, disks_uuids with
+      | None, [] ->
+          error (f_"there must be ‘-oo rhv-disk-uuid’ parameters passed on
the command line to specify the UUIDs of guest disk images (for this guest:
%d)")
+            (List.length targets)
+      | Some uuids, _ -> uuids
+      | None, uuids -> uuids in
+    assert (List.length image_uuids = List.length targets);
 
     (* The storage domain UUID. *)
     let sd_uuid @@ -433,7 +466,7 @@ If the messages above are not sufficient to
diagnose the problem then add the
     let ovf        Create_ovf.create_ovf source targets guestcaps inspect
                             target_firmware output_alloc
-                            sd_uuid disks_uuids vol_uuids vm_uuid
+                            sd_uuid image_uuids vol_uuids vm_uuid
                             OVirt in
     let ovf = DOM.doc_to_string ovf in
 
diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
index 685680213..896c17942 100644
--- a/v2v/rhv-upload-plugin.py
+++ b/v2v/rhv-upload-plugin.py
@@ -135,6 +135,8 @@ def open(readonly):
         disk_format = types.DiskFormat.COW
     disk = disks_service.add(
         disk = types.Disk(
+            # The ID is optional.
+            id = params.get('rhv_disk_uuid'),
             name = params['disk_name'],
             description = "Uploaded by virt-v2v",
             format = disk_format,
diff --git a/v2v/virt-v2v-output-rhv.pod b/v2v/virt-v2v-output-rhv.pod
index 651f61dae..c91477f62 100644
--- a/v2v/virt-v2v-output-rhv.pod
+++ b/v2v/virt-v2v-output-rhv.pod
@@ -9,6 +9,7 @@ virt-v2v-output-rhv - Using virt-v2v to convert guests to oVirt
or RHV
                         [-oo rhv-cafile=FILE]
                         [-oo rhv-cluster=CLUSTER]
                         [-oo rhv-direct]
+                        [-oo rhv-disk-uuid=UUID ...]
                         [-oo rhv-verifypeer]
 
  virt-v2v [-i* options] -o rhv -os [esd:/path|/path]
@@ -104,6 +105,28 @@ F</etc/pki/ovirt-engine/ca.pem> on the oVirt engine.
 
 Set the RHV Cluster Name.  If not given it uses C<Default>.
 
+=item I<-oo rhv-disk-uuid=>C<UUID>
+
+This option is used to specify UUIDs for the disks when creating the
+virtual machine.  However, please note that:
+
+=over 4
+
+=item *
+
+you B<must> pass as many I<-oo rhv-disk-uuid=UUID> options as the
+amount of disks in the guest
+
+=item *
+
+the specified UUIDs are used as they are, without checking whether
+they are already used by other disks
+
+=back
+
+This option is considered advanced, and to be used mostly in
+combination with I<--no-copy>.
+
 =item I<-oo rhv-direct>
 
 If this option is given then virt-v2v will attempt to directly upload
-- 
2.21.0

On Thu, Sep 19, 2019 at 12:38:41PM +0200, Pino Toscano
wrote:
>This way it is possible to override the UUIDs of the uploaded disks,
>instead of letting RHV generate them.
>
>This can be useful to force certain UUIDs, and to specify the disks in
>--no-copy mode (which now can be used).
>---
> v2v/output_rhv_upload.ml    | 43 ++++++++++++++++++++++++++++++++-----
> v2v/rhv-upload-plugin.py    |  2 ++
> v2v/virt-v2v-output-rhv.pod | 23 ++++++++++++++++++++
> 3 files changed, 63 insertions(+), 5 deletions(-)
>
>diff --git a/v2v/virt-v2v-output-rhv.pod b/v2v/virt-v2v-output-rhv.pod
>index 651f61dae..c91477f62 100644
>--- a/v2v/virt-v2v-output-rhv.pod
>+++ b/v2v/virt-v2v-output-rhv.pod
>@@ -9,6 +9,7 @@ virt-v2v-output-rhv - Using virt-v2v to convert guests to
oVirt or RHV
>                         [-oo rhv-cafile=FILE]
>                         [-oo rhv-cluster=CLUSTER]
>                         [-oo rhv-direct]
>+                        [-oo rhv-disk-uuid=UUID ...]
>                         [-oo rhv-verifypeer]
>
>  virt-v2v [-i* options] -o rhv -os [esd:/path|/path]
>@@ -104,6 +105,28 @@ F</etc/pki/ovirt-engine/ca.pem> on the oVirt
engine.
>
> Set the RHV Cluster Name.  If not given it uses C<Default>.
>
>+=item I<-oo rhv-disk-uuid=>C<UUID>
>+
>+This option is used to specify UUIDs for the disks when creating the
>+virtual machine.  However, please note that:
>+
Please add some information that this is optional.  This way it sounds like it
is mandatory.

ACK with that.  It is way better implementation than I have.

On Thursday, 19 September 2019 13:16:01 CEST Martin Kletzander
wrote:
> On Thu, Sep 19, 2019 at 12:38:41PM +0200, Pino Toscano wrote:
> >This way it is possible to override the UUIDs of the uploaded disks,
> >instead of letting RHV generate them.
> >
> >This can be useful to force certain UUIDs, and to specify the disks in
> >--no-copy mode (which now can be used).
> >---
> > v2v/output_rhv_upload.ml    | 43 ++++++++++++++++++++++++++++++++-----
> > v2v/rhv-upload-plugin.py    |  2 ++
> > v2v/virt-v2v-output-rhv.pod | 23 ++++++++++++++++++++
> > 3 files changed, 63 insertions(+), 5 deletions(-)
> >
> >diff --git a/v2v/virt-v2v-output-rhv.pod b/v2v/virt-v2v-output-rhv.pod
> >index 651f61dae..c91477f62 100644
> >--- a/v2v/virt-v2v-output-rhv.pod
> >+++ b/v2v/virt-v2v-output-rhv.pod
> >@@ -9,6 +9,7 @@ virt-v2v-output-rhv - Using virt-v2v to convert guests
to oVirt or RHV
> >                         [-oo rhv-cafile=FILE]
> >                         [-oo rhv-cluster=CLUSTER]
> >                         [-oo rhv-direct]
> >+                        [-oo rhv-disk-uuid=UUID ...]
> >                         [-oo rhv-verifypeer]
> >
> >  virt-v2v [-i* options] -o rhv -os [esd:/path|/path]
> >@@ -104,6 +105,28 @@ F</etc/pki/ovirt-engine/ca.pem> on the oVirt
engine.
> >
> > Set the RHV Cluster Name.  If not given it uses C<Default>.
> >
> >+=item I<-oo rhv-disk-uuid=>C<UUID>
> >+
> >+This option is used to specify UUIDs for the disks when creating the
> >+virtual machine.  However, please note that:
> >+
> 
> Please add some information that this is optional.  This way it sounds like
it
> is mandatory.
What about the following:

  This option can used to manually specify UUIDs for the disks when
  creating the virtual machine.  If not specified, the oVirt engine will
  generate random UUIDs for the disks.  Please note that:
  [etc]

-- 
Pino Toscano