Libguestfs - Oct 2019 - Re: [PATCH] v2v: Output saved overlays in a machine-readable fashion

Even though this option is not to be used according to the manual, it:

 a) still might be useful even for machine-readable logs

 b) should not break the machine-readable output

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 v2v/v2v.ml | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/v2v/v2v.ml b/v2v/v2v.ml
index 4ee15663f261..508a2b4f39a5 100644
--- a/v2v/v2v.ml
+++ b/v2v/v2v.ml
@@ -815,13 +815,28 @@ and actual_target_size target_file disk_stats  
 (* Save overlays if --debug-overlays option was used. *)
 and preserve_overlays overlays src_name -  List.iter (
-    fun ov ->
-      let saved_filename -        sprintf "%s/%s-%s.qcow2"
overlay_dir src_name ov.ov_sd in
-      rename ov.ov_overlay_file saved_filename;
-      info (f_"Overlay saved as %s [--debug-overlays]")
saved_filename
-  ) overlays
+  let filenames = List.map (
+        fun ov ->
+          let saved_filename +            sprintf "%s/%s-%s.qcow2"
overlay_dir src_name ov.ov_sd in
+          rename ov.ov_overlay_file saved_filename;
+          saved_filename
+      ) overlays in
+  match machine_readable () with
+  | None ->
+    List.iter (
+      fun filename ->
+        info (f_"Overlay saved as %s [--debug-overlays]") filename
+    ) filenames
+  | Some {pr} ->
+    let json = [
+      "type", JSON.String "data";
+      "data", JSON.Dict [
+        "saved_overlays",
+        JSON.List (List.map (fun s -> JSON.String s) filenames);
+      ]
+    ] in
+    pr "%s\n" (JSON.string_of_doc json)
 
 (* Request guest caps based on source configuration. *)
 and rcaps_from_source source -- 
2.23.0

On Wed, Oct 09, 2019 at 02:19:46PM +0200, Martin Kletzander
wrote:
>Even though this option is not to be used according to the manual, it:
>
> a) still might be useful even for machine-readable logs
>
> b) should not break the machine-readable output
>
I should've mentioned "which it currently does if it is being parsed
from
stdout".  But I would love to hear opinions on whether it could be
considered
making this (or new parameter) noted as stable for this particular purpose?  It
looks very useful from the usage of it.  I can expand on that if anyone wants.

On Wed, Oct 09, 2019 at 02:19:46PM +0200, Martin Kletzander
wrote:
> Even though this option is not to be used according to the manual, it:
> 
>  a) still might be useful even for machine-readable logs
> 
>  b) should not break the machine-readable output
I'm a bit confused what you're trying to do here.

Rich.
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  v2v/v2v.ml | 29 ++++++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/v2v/v2v.ml b/v2v/v2v.ml
> index 4ee15663f261..508a2b4f39a5 100644
> --- a/v2v/v2v.ml
> +++ b/v2v/v2v.ml
> @@ -815,13 +815,28 @@ and actual_target_size target_file disk_stats >  
>  (* Save overlays if --debug-overlays option was used. *)
>  and preserve_overlays overlays src_name > -  List.iter (
> -    fun ov ->
> -      let saved_filename > -        sprintf "%s/%s-%s.qcow2"
overlay_dir src_name ov.ov_sd in
> -      rename ov.ov_overlay_file saved_filename;
> -      info (f_"Overlay saved as %s [--debug-overlays]")
saved_filename
> -  ) overlays
> +  let filenames = List.map (
> +        fun ov ->
> +          let saved_filename > +            sprintf
"%s/%s-%s.qcow2" overlay_dir src_name ov.ov_sd in
> +          rename ov.ov_overlay_file saved_filename;
> +          saved_filename
> +      ) overlays in
> +  match machine_readable () with
> +  | None ->
> +    List.iter (
> +      fun filename ->
> +        info (f_"Overlay saved as %s [--debug-overlays]")
filename
> +    ) filenames
> +  | Some {pr} ->
> +    let json = [
> +      "type", JSON.String "data";
> +      "data", JSON.Dict [
> +        "saved_overlays",
> +        JSON.List (List.map (fun s -> JSON.String s) filenames);
> +      ]
> +    ] in
> +    pr "%s\n" (JSON.string_of_doc json)
>  
>  (* Request guest caps based on source configuration. *)
>  and rcaps_from_source source > -- 
> 2.23.0
-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

On Thu, Oct 10, 2019 at 03:33:25PM +0100, Richard W.M. Jones
wrote:
>On Wed, Oct 09, 2019 at 02:19:46PM +0200, Martin Kletzander wrote:
>> Even though this option is not to be used according to the manual, it:
>>
>>  a) still might be useful even for machine-readable logs
>>
>>  b) should not break the machine-readable output
>
>I'm a bit confused what you're trying to do here.
>
When working on a conversion where the disk is already copied from the source
hypervisor to the destination storage (due to various reasons, e.g. having
faster access to one of the storages or being able to use the disk_sync script
to prefetch some of the data before the machine gets shut down) I need to
utilize virt-v2v for two specific purposes:

 1) to create the output metadata and

 2) to make the changes in the guest disks (which are already copied)

but not for the third thing that virt-v2v does as well, which is copying the
disks from source to destination.

For case #1 I initially used virt-v2v --no-copy, which creates the metadata, but
discards the changes made to the disks in the overlays and for #2 I used
--in-place which changes the data on the source (for which I crafted a specific
XML to be used with -i libvirtxml), but does not output any metadata for the
destination.

There are couple of issues I am facing with this:

 a) I need to run the conversion twice

 b) --in-place does not know about the destination (and it does not do the same
    thing as if it knew)

 c) It should only be done using --no-copy first and --in-place second just to
    make sure the modifications are not done twice (although this is just to
    stay on the safe side).

It still felt like a massive hack to me and while I still held on to the idea of
keeping the overlays around without actually copying the data anywhere I found
the --debug-overlays option.  It fits my use case perfectly because what I am
doing now (with this patch) is:

 i) run virt-v2v --no-copy with the proper destination

 ii) take the overlays and run qemu-img commit on them

 iii) remove the overlays

This way all of my concerns are solved and the only one that arises is the fact
that --debug-overlays is documented as "... only used for debugging and may
be
removed in a future version."

One interesting thing is that I only found that information in the man page
after I wrote this patch and started using it.  The reason for it was that I
started actually writing something very similar to this and I stumbled upon the
code that already does that (with --debug-overlays).

I also thought that if someone runs virt-v2v --machine-readable --debug-overlays
that the machine readable output would be broken because the output is not json,
but it looks like (I'm just trying it now) --machine-readable does not write
any
data to stdout or stderr, even when specified as
--machine-readable=stream:stdout (or stderr) for simple libvirt2libvirt
migration.  So point (b) from the commit message is probably invalid.  Oh,
actually, I don't see it being used for anything else than printing
apabilities and estimates.  I wonder where the json messages in machine readable
logfile that virt-v2v-wrapper requests are coming from.

One last thing is that currently any application that wants to run virt-v2v
(virt-v2v-wrapper in my case) needs to parse the output to get any meaningful
data from it.  What I tried here was using a new type of message called
'data' that upper applications could just parse and use.  In the long
run I could add more data to the output so that it is easier to use from
virt-v2v-wrapper and other applications.  But that is not something that is
necessary for this particular patch (or the discussion about the
--debug-overlays being supported in the long term), it just made it easier.

Does that make sense?

For some more context/info, see How I used it in a PoC here:

 
https://github.com/nertpinx/v2v-conversion-host/blob/asdf/wrapper/log_parser.py#L71
>Rich.
>
>> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
>> ---
>>  v2v/v2v.ml | 29 ++++++++++++++++++++++-------
>>  1 file changed, 22 insertions(+), 7 deletions(-)
>>
>> diff --git a/v2v/v2v.ml b/v2v/v2v.ml
>> index 4ee15663f261..508a2b4f39a5 100644
>> --- a/v2v/v2v.ml
>> +++ b/v2v/v2v.ml
>> @@ -815,13 +815,28 @@ and actual_target_size target_file disk_stats
>>
>>  (* Save overlays if --debug-overlays option was used. *)
>>  and preserve_overlays overlays src_name >> -  List.iter (
>> -    fun ov ->
>> -      let saved_filename >> -        sprintf
"%s/%s-%s.qcow2" overlay_dir src_name ov.ov_sd in
>> -      rename ov.ov_overlay_file saved_filename;
>> -      info (f_"Overlay saved as %s [--debug-overlays]")
saved_filename
>> -  ) overlays
>> +  let filenames = List.map (
>> +        fun ov ->
>> +          let saved_filename >> +            sprintf
"%s/%s-%s.qcow2" overlay_dir src_name ov.ov_sd in
>> +          rename ov.ov_overlay_file saved_filename;
>> +          saved_filename
>> +      ) overlays in
>> +  match machine_readable () with
>> +  | None ->
>> +    List.iter (
>> +      fun filename ->
>> +        info (f_"Overlay saved as %s [--debug-overlays]")
filename
>> +    ) filenames
>> +  | Some {pr} ->
>> +    let json = [
>> +      "type", JSON.String "data";
>> +      "data", JSON.Dict [
>> +        "saved_overlays",
>> +        JSON.List (List.map (fun s -> JSON.String s) filenames);
>> +      ]
>> +    ] in
>> +    pr "%s\n" (JSON.string_of_doc json)
>>
>>  (* Request guest caps based on source configuration. *)
>>  and rcaps_from_source source >> --
>> 2.23.0
>
>-- 
>Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
>Read my programming and virtualization blog: http://rwmj.wordpress.com
>virt-top is 'top' for virtual machines.  Tiny program with many
>powerful monitoring features, net stats, disk stats, logging, etc.
>http://people.redhat.com/~rjones/virt-top