Libguestfs - Mar 2019 - [PATCH 3/4] common/mltools: allow fd for machine readable output

Enhance the output in machine parseable mode, by outputting all the
messages of OCaml tools as JSON to the machine parseable stream.

Related, although not strictly needed for this (and thus can be split
if requested), is the addition of the fd format for the machine
readable stream.

Pino Toscano (4):
  common/mltools: move the code for machine readable up
  common/mltools: make sure machine readable output is flushed
  common/mltools: allow fd for machine readable output
  OCaml tools: output messages into JSON for machine readable

 common/mltools/Makefile.am     |  2 +-
 common/mltools/tools_utils-c.c | 68 ++++++++++++++++++++++++++
 common/mltools/tools_utils.ml  | 88 ++++++++++++++++++++++------------
 lib/guestfs.pod                | 24 ++++++++++
 4 files changed, 151 insertions(+), 31 deletions(-)

-- 
2.20.1

Move the code for handling machine readable up in the file, so it can be
used by other functions.

Only code motion, no behaviour changes.
---
 common/mltools/tools_utils.ml | 60 +++++++++++++++++------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
index 24641369e..5a35708cd 100644
--- a/common/mltools/tools_utils.ml
+++ b/common/mltools/tools_utils.ml
@@ -33,6 +33,36 @@ external c_inspect_decrypt : Guestfs.t -> int64 ->
(string * key_store_key) list
 external c_set_echo_keys : unit -> unit =
"guestfs_int_mllib_set_echo_keys" "noalloc"
 external c_set_keys_from_stdin : unit -> unit =
"guestfs_int_mllib_set_keys_from_stdin" "noalloc"
 
+type machine_readable_fn = {
+  pr : 'a. ('a, unit, string, unit) format4 -> 'a;
+} (* [@@unboxed] *)
+
+type machine_readable_output_type +  | NoOutput
+  | Channel of out_channel
+  | File of string
+let machine_readable_output = ref NoOutput
+let machine_readable_channel = ref None
+let machine_readable () +  let chan +    if !machine_readable_channel = None
then (
+      let chan +        match !machine_readable_output with
+        | NoOutput -> None
+        | Channel chan -> Some chan
+        | File f -> Some (open_out f) in
+      machine_readable_channel := chan
+    );
+    !machine_readable_channel
+  in
+  match chan with
+  | None -> None
+  | Some chan ->
+    let pr fs +      ksprintf (output_string chan) fs
+    in
+    Some { pr }
+
 (* ANSI terminal colours. *)
 let istty chan    Unix.isatty (Unix.descr_of_out_channel chan)
@@ -236,36 +266,6 @@ let human_size i      )
   )
 
-type machine_readable_fn = {
-  pr : 'a. ('a, unit, string, unit) format4 -> 'a;
-} (* [@@unboxed] *)
-
-type machine_readable_output_type -  | NoOutput
-  | Channel of out_channel
-  | File of string
-let machine_readable_output = ref NoOutput
-let machine_readable_channel = ref None
-let machine_readable () -  let chan -    if !machine_readable_channel = None
then (
-      let chan -        match !machine_readable_output with
-        | NoOutput -> None
-        | Channel chan -> Some chan
-        | File f -> Some (open_out f) in
-      machine_readable_channel := chan
-    );
-    !machine_readable_channel
-  in
-  match chan with
-  | None -> None
-  | Some chan ->
-    let pr fs -      ksprintf (output_string chan) fs
-    in
-    Some { pr }
-
 type cmdline_options = {
   getopt : Getopt.t;
   ks : key_store;
-- 
2.20.1

Enhance the helper printf function for machine readable output to always
flush after each string: this way, readers of the machine readable
stream can get the output as soon as it is outputted.
---
 common/mltools/tools_utils.ml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
index 5a35708cd..ade4cb37f 100644
--- a/common/mltools/tools_utils.ml
+++ b/common/mltools/tools_utils.ml
@@ -59,7 +59,11 @@ let machine_readable ()    | None -> None
   | Some chan ->
     let pr fs -      ksprintf (output_string chan) fs
+      let out s +        output_string chan s;
+        flush chan
+      in
+      ksprintf out fs
     in
     Some { pr }
 
-- 
2.20.1

Allow to specify a file descriptor for the machine readable output.

Sadly, the OCaml C glue for the channels is not public API, so enable
the internals for this...
---
 common/mltools/tools_utils-c.c | 17 +++++++++++++++++
 common/mltools/tools_utils.ml  | 10 +++++++++-
 lib/guestfs.pod                |  5 +++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/common/mltools/tools_utils-c.c b/common/mltools/tools_utils-c.c
index c88c95082..553aa6631 100644
--- a/common/mltools/tools_utils-c.c
+++ b/common/mltools/tools_utils-c.c
@@ -29,6 +29,9 @@
 #include <caml/memory.h>
 #include <caml/mlvalues.h>
 #include <caml/unixsupport.h>
+/* Evil ... */
+#define CAML_INTERNALS
+#include <caml/io.h>
 
 #include <guestfs.h>
 
@@ -37,6 +40,7 @@
 extern value guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value
keysv);
 extern value guestfs_int_mllib_set_echo_keys (value unitv);
 extern value guestfs_int_mllib_set_keys_from_stdin (value unitv);
+extern value guestfs_int_mllib_open_out_channel_from_fd (value fdv);
 
 /* Interface with the guestfish inspection and decryption code. */
 int echo_keys = 0;
@@ -103,3 +107,16 @@ guestfs_int_mllib_set_keys_from_stdin (value unitv)
   keys_from_stdin = 1;
   return Val_unit;
 }
+
+value
+guestfs_int_mllib_open_out_channel_from_fd (value fdv)
+{
+  CAMLparam1 (fdv);
+  struct channel *chan;
+
+  chan = caml_open_descriptor_out (Int_val (fdv));
+  if (!chan)
+    caml_raise_out_of_memory ();
+
+  CAMLreturn (caml_alloc_channel (chan));
+}
diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
index ade4cb37f..3c54cd4a0 100644
--- a/common/mltools/tools_utils.ml
+++ b/common/mltools/tools_utils.ml
@@ -32,6 +32,7 @@ and key_store_key  external c_inspect_decrypt : Guestfs.t
-> int64 -> (string * key_store_key) list -> unit =
"guestfs_int_mllib_inspect_decrypt"
 external c_set_echo_keys : unit -> unit =
"guestfs_int_mllib_set_echo_keys" "noalloc"
 external c_set_keys_from_stdin : unit -> unit =
"guestfs_int_mllib_set_keys_from_stdin" "noalloc"
+external c_out_channel_from_fd : int -> out_channel =
"guestfs_int_mllib_open_out_channel_from_fd"
 
 type machine_readable_fn = {
   pr : 'a. ('a, unit, string, unit) format4 -> 'a;
@@ -41,6 +42,7 @@ type machine_readable_output_type    | NoOutput
   | Channel of out_channel
   | File of string
+  | Fd of int
 let machine_readable_output = ref NoOutput
 let machine_readable_channel = ref None
 let machine_readable () @@ -50,7 +52,8 @@ let machine_readable ()         
match !machine_readable_output with
         | NoOutput -> None
         | Channel chan -> Some chan
-        | File f -> Some (open_out f) in
+        | File f -> Some (open_out f)
+        | Fd fd -> Some (c_out_channel_from_fd fd) in
       machine_readable_channel := chan
     );
     !machine_readable_channel
@@ -296,6 +299,11 @@ let create_standard_options argspec ?anon_fun ?(key_opts =
false) ?(machine_read
           | n ->
             error (f_"invalid output stream for --machine-readable:
%s") fmt in
         machine_readable_output := Channel chan
+      | "fd" ->
+        (try
+          machine_readable_output := Fd (int_of_string outname)
+        with Failure _ ->
+          error (f_"invalid output fd for --machine-readable: %s")
fmt)
       | n ->
         error (f_"invalid output for --machine-readable: %s") fmt
       )
diff --git a/lib/guestfs.pod b/lib/guestfs.pod
index 53cece2da..f11028466 100644
--- a/lib/guestfs.pod
+++ b/lib/guestfs.pod
@@ -3287,6 +3287,11 @@ The possible values are:
 
 =over 4
 
+=item B<fd:>I<fd>
+
+The output goes to the specified I<fd>, which is a file descriptor
+already opened for writing.
+
 =item B<file:>F<filename>
 
 The output goes to the specified F<filename>.
-- 
2.20.1

When the machine readable mode is enabled, print all the messages
(progress, info, warning, and errors) also as JSON in the machine
readable stream: this way, users can easily parse the status of the
OCaml tool, and report that back.

The formatting of the current date time into the RFC 3999 format is done
in C, because of the lack of OCaml APIs for this.
---
 common/mltools/Makefile.am     |  2 +-
 common/mltools/tools_utils-c.c | 51 ++++++++++++++++++++++++++++++++++
 common/mltools/tools_utils.ml  | 16 +++++++++++
 lib/guestfs.pod                | 19 +++++++++++++
 4 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/common/mltools/Makefile.am b/common/mltools/Makefile.am
index 37d10e610..ee8c319fd 100644
--- a/common/mltools/Makefile.am
+++ b/common/mltools/Makefile.am
@@ -45,12 +45,12 @@ SOURCES_MLI = \
 
 SOURCES_ML = \
 	getopt.ml \
+	JSON.ml \
 	tools_utils.ml \
 	URI.ml \
 	planner.ml \
 	registry.ml \
 	regedit.ml \
-	JSON.ml \
 	JSON_parser.ml \
 	curl.ml \
 	checksums.ml \
diff --git a/common/mltools/tools_utils-c.c b/common/mltools/tools_utils-c.c
index 553aa6631..977f932d9 100644
--- a/common/mltools/tools_utils-c.c
+++ b/common/mltools/tools_utils-c.c
@@ -23,6 +23,8 @@
 #include <unistd.h>
 #include <errno.h>
 #include <error.h>
+#include <time.h>
+#include <string.h>
 
 #include <caml/alloc.h>
 #include <caml/fail.h>
@@ -41,6 +43,7 @@ extern value guestfs_int_mllib_inspect_decrypt (value gv,
value gpv, value keysv
 extern value guestfs_int_mllib_set_echo_keys (value unitv);
 extern value guestfs_int_mllib_set_keys_from_stdin (value unitv);
 extern value guestfs_int_mllib_open_out_channel_from_fd (value fdv);
+extern value guestfs_int_mllib_rfc3999_date_time_string (value unitv);
 
 /* Interface with the guestfish inspection and decryption code. */
 int echo_keys = 0;
@@ -120,3 +123,51 @@ guestfs_int_mllib_open_out_channel_from_fd (value fdv)
 
   CAMLreturn (caml_alloc_channel (chan));
 }
+
+value
+guestfs_int_mllib_rfc3999_date_time_string (value unitv)
+{
+  CAMLparam1 (unitv);
+  char buf[64];
+  struct timespec ts;
+  struct tm tm;
+  size_t ret;
+  size_t total = 0;
+
+  if (clock_gettime (CLOCK_REALTIME, &ts) == -1)
+    unix_error (errno, (char *) "clock_gettime", Val_unit);
+
+  if (localtime_r (&ts.tv_sec, &tm) == NULL)
+    unix_error (errno, (char *) "localtime_r", caml_copy_int64
(ts.tv_sec));
+
+  /* Sadly strftime does not support nanoseconds, so what we do is:
+   * - stringify everything before the nanoseconds
+   * - print the nanoseconds
+   * - stringify the rest (i.e. the timezone)
+   * then place ':' between the hours, and the minutes of the
+   * timezone offset.
+   */
+
+  ret = strftime (buf, sizeof (buf), "%Y-%m-%dT%H:%M:%S.", &tm);
+  if (ret == 0)
+    unix_error (errno, (char *) "strftime", Val_unit);
+  total += ret;
+
+  ret = snprintf (buf + total, sizeof (buf) - total, "%09ld",
ts.tv_nsec);
+  if (ret == 0)
+    unix_error (errno, (char *) "sprintf", caml_copy_int64
(ts.tv_nsec));
+  total += ret;
+
+  ret = strftime (buf + total, sizeof (buf) - total, "%z", &tm);
+  if (ret == 0)
+    unix_error (errno, (char *) "strftime", Val_unit);
+  total += ret;
+
+  /* Move the timezone minutes one character to the right, moving the
+   * null character too.
+   */
+  memmove (buf + total - 1, buf + total - 2, 3);
+  buf[total - 2] = ':';
+
+  CAMLreturn (caml_copy_string (buf));
+}
diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
index 3c54cd4a0..1a1d11075 100644
--- a/common/mltools/tools_utils.ml
+++ b/common/mltools/tools_utils.ml
@@ -33,6 +33,7 @@ external c_inspect_decrypt : Guestfs.t -> int64 ->
(string * key_store_key) list
 external c_set_echo_keys : unit -> unit =
"guestfs_int_mllib_set_echo_keys" "noalloc"
 external c_set_keys_from_stdin : unit -> unit =
"guestfs_int_mllib_set_keys_from_stdin" "noalloc"
 external c_out_channel_from_fd : int -> out_channel =
"guestfs_int_mllib_open_out_channel_from_fd"
+external c_rfc3999_date_time_string : unit -> string =
"guestfs_int_mllib_rfc3999_date_time_string"
 
 type machine_readable_fn = {
   pr : 'a. ('a, unit, string, unit) format4 -> 'a;
@@ -85,12 +86,24 @@ let ansi_magenta ?(chan = stdout) ()  let ansi_restore
?(chan = stdout) ()    if colours () || istty chan then output_string chan
"\x1b[0m"
 
+let log_as_json msgtype msg +  match machine_readable () with
+  | None -> ()
+  | Some { pr } ->
+    let json = [
+      "message", JSON.String msg;
+      "timestamp", JSON.String (c_rfc3999_date_time_string ());
+      "type", JSON.String msgtype;
+    ] in
+    pr "%s\n" (JSON.string_of_doc ~fmt:JSON.Compact json)
+
 (* Timestamped progress messages, used for ordinary messages when not
  * --quiet.
  *)
 let start_t = Unix.gettimeofday ()
 let message fs    let display str +    log_as_json "message" str;
     if not (quiet ()) then (
       let t = sprintf "%.1f" (Unix.gettimeofday () -. start_t) in
       printf "[%6s] " t;
@@ -105,6 +118,7 @@ let message fs  (* Error messages etc. *)
 let error ?(exit_code = 1) fs    let display str +    log_as_json
"error" str;
     let chan = stderr in
     ansi_red ~chan ();
     wrap ~chan (sprintf (f_"%s: error: %s") prog str);
@@ -123,6 +137,7 @@ let error ?(exit_code = 1) fs  
 let warning fs    let display str +    log_as_json "warning" str;
     let chan = stdout in
     ansi_blue ~chan ();
     wrap ~chan (sprintf (f_"%s: warning: %s") prog str);
@@ -133,6 +148,7 @@ let warning fs  
 let info fs    let display str +    log_as_json "info" str;
     let chan = stdout in
     ansi_magenta ~chan ();
     wrap ~chan (sprintf (f_"%s: %s") prog str);
diff --git a/lib/guestfs.pod b/lib/guestfs.pod
index f11028466..3c1d635c5 100644
--- a/lib/guestfs.pod
+++ b/lib/guestfs.pod
@@ -3279,6 +3279,25 @@ Some of the tools support a I<--machine-readable>
option, which is
 generally used to make the output more machine friendly, for easier
 parsing for example.  By default, this output goes to stdout.
 
+When using the I<--machine-readable> option, the progress,
+information, warning, and error messages are also printed in JSON
+format for easier log tracking.  Thus, it is highly recommended to
+redirect the machine-readable output to a different stream.  The
+format of these JSON messages is like the following (actually printed
+within a single line, below it is indented for readability):
+
+ {
+   "message": "Finishing off",
+   "timestamp": "2019-03-22T14:46:49.067294446+01:00",
+   "type": "message"
+ }
+
+C<type> can be: C<message> for progress messages, C<info> for
+information messages, C<warning> for warning messages, and C<error>
+for error message.
+C<timestamp> is the L<RFC
3999|https://www.ietf.org/rfc/rfc3339.txt>
+timestamp of the message.
+
 In addition to that, a subset of these tools support an extra string
 passed to the I<--machine-readable> option: this string specifies
 where the machine-readable output will go.
-- 
2.20.1

On Fri, Mar 22, 2019 at 04:33:41PM +0100, Pino Toscano
wrote:
> Enhance the helper printf function for machine readable output to always
> flush after each string: this way, readers of the machine readable
> stream can get the output as soon as it is outputted.
> ---
>  common/mltools/tools_utils.ml | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
> index 5a35708cd..ade4cb37f 100644
> --- a/common/mltools/tools_utils.ml
> +++ b/common/mltools/tools_utils.ml
> @@ -59,7 +59,11 @@ let machine_readable () >    | None -> None
>    | Some chan ->
>      let pr fs > -      ksprintf (output_string chan) fs
> +      let out s > +        output_string chan s;
> +        flush chan
> +      in
> +      ksprintf out fs
>      in
>      Some { pr }
I was looking to see if any of the existing calls to machine_readable
are using %! (a format directive which causes the channel to be
flushed).  I had expected there would be some, but my simple "git
grep"s aren't turning up anything right now.

Anyway, ACK 1/4 and 2/4.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

On Fri, Mar 22, 2019 at 04:33:42PM +0100, Pino Toscano
wrote:
> Allow to specify a file descriptor for the machine readable output.
> 
> Sadly, the OCaml C glue for the channels is not public API, so enable
> the internals for this...
> ---
>  common/mltools/tools_utils-c.c | 17 +++++++++++++++++
>  common/mltools/tools_utils.ml  | 10 +++++++++-
>  lib/guestfs.pod                |  5 +++++
>  3 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/common/mltools/tools_utils-c.c
b/common/mltools/tools_utils-c.c
> index c88c95082..553aa6631 100644
> --- a/common/mltools/tools_utils-c.c
> +++ b/common/mltools/tools_utils-c.c
> @@ -29,6 +29,9 @@
>  #include <caml/memory.h>
>  #include <caml/mlvalues.h>
>  #include <caml/unixsupport.h>
> +/* Evil ... */
> +#define CAML_INTERNALS
> +#include <caml/io.h>
I think this is not necessary.  On Unix-like platforms,
Unix.file_descr is really just an int, so you can cast between the two
using Val_int/Int_val (from C) or Obj.magic (from OCaml).  Note we
already rely on this in libguestfs, in
v2v/qemuopts-c.c:guestfs_int_qemuopts_to_chan

So I think this should work:

  | Fd of Unix.file_descr

...

  | Fd fd -> Some (Unix.out_channel_of_descr fd)

...

  | "fd" ->
    machine_readable_output := Fd (Obj.magic (int_of_string outname))

If you don't want to use Obj.magic, then something using /dev/fd/<N>
is another possibility.

Rich.
>  
>  #include <guestfs.h>
>  
> @@ -37,6 +40,7 @@
>  extern value guestfs_int_mllib_inspect_decrypt (value gv, value gpv, value
keysv);
>  extern value guestfs_int_mllib_set_echo_keys (value unitv);
>  extern value guestfs_int_mllib_set_keys_from_stdin (value unitv);
> +extern value guestfs_int_mllib_open_out_channel_from_fd (value fdv);
>  
>  /* Interface with the guestfish inspection and decryption code. */
>  int echo_keys = 0;
> @@ -103,3 +107,16 @@ guestfs_int_mllib_set_keys_from_stdin (value unitv)
>    keys_from_stdin = 1;
>    return Val_unit;
>  }
> +
> +value
> +guestfs_int_mllib_open_out_channel_from_fd (value fdv)
> +{
> +  CAMLparam1 (fdv);
> +  struct channel *chan;
> +
> +  chan = caml_open_descriptor_out (Int_val (fdv));
> +  if (!chan)
> +    caml_raise_out_of_memory ();
> +
> +  CAMLreturn (caml_alloc_channel (chan));
> +}
> diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
> index ade4cb37f..3c54cd4a0 100644
> --- a/common/mltools/tools_utils.ml
> +++ b/common/mltools/tools_utils.ml
> @@ -32,6 +32,7 @@ and key_store_key >  external c_inspect_decrypt :
Guestfs.t -> int64 -> (string * key_store_key) list -> unit =
"guestfs_int_mllib_inspect_decrypt"
>  external c_set_echo_keys : unit -> unit =
"guestfs_int_mllib_set_echo_keys" "noalloc"
>  external c_set_keys_from_stdin : unit -> unit =
"guestfs_int_mllib_set_keys_from_stdin" "noalloc"
> +external c_out_channel_from_fd : int -> out_channel =
"guestfs_int_mllib_open_out_channel_from_fd"
>  
>  type machine_readable_fn = {
>    pr : 'a. ('a, unit, string, unit) format4 -> 'a;
> @@ -41,6 +42,7 @@ type machine_readable_output_type >    | NoOutput
>    | Channel of out_channel
>    | File of string
> +  | Fd of int
>  let machine_readable_output = ref NoOutput
>  let machine_readable_channel = ref None
>  let machine_readable () > @@ -50,7 +52,8 @@ let machine_readable ()
>          match !machine_readable_output with
>          | NoOutput -> None
>          | Channel chan -> Some chan
> -        | File f -> Some (open_out f) in
> +        | File f -> Some (open_out f)
> +        | Fd fd -> Some (c_out_channel_from_fd fd) in
>        machine_readable_channel := chan
>      );
>      !machine_readable_channel
> @@ -296,6 +299,11 @@ let create_standard_options argspec ?anon_fun
?(key_opts = false) ?(machine_read
>            | n ->
>              error (f_"invalid output stream for --machine-readable:
%s") fmt in
>          machine_readable_output := Channel chan
> +      | "fd" ->
> +        (try
> +          machine_readable_output := Fd (int_of_string outname)
> +        with Failure _ ->
> +          error (f_"invalid output fd for --machine-readable:
%s") fmt)
>        | n ->
>          error (f_"invalid output for --machine-readable: %s")
fmt
>        )
> diff --git a/lib/guestfs.pod b/lib/guestfs.pod
> index 53cece2da..f11028466 100644
> --- a/lib/guestfs.pod
> +++ b/lib/guestfs.pod
> @@ -3287,6 +3287,11 @@ The possible values are:
>  
>  =over 4
>  
> +=item B<fd:>I<fd>
> +
> +The output goes to the specified I<fd>, which is a file descriptor
> +already opened for writing.
> +
>  =item B<file:>F<filename>
>  
>  The output goes to the specified F<filename>.
> -- 
> 2.20.1
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs
-- 
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/

On Fri, Mar 22, 2019 at 04:33:43PM +0100, Pino Toscano
wrote:
> When the machine readable mode is enabled, print all the messages
> (progress, info, warning, and errors) also as JSON in the machine
> readable stream: this way, users can easily parse the status of the
> OCaml tool, and report that back.
> 
> The formatting of the current date time into the RFC 3999 format is done
> in C, because of the lack of OCaml APIs for this.
> ---
>  common/mltools/Makefile.am     |  2 +-
>  common/mltools/tools_utils-c.c | 51 ++++++++++++++++++++++++++++++++++
>  common/mltools/tools_utils.ml  | 16 +++++++++++
>  lib/guestfs.pod                | 19 +++++++++++++
>  4 files changed, 87 insertions(+), 1 deletion(-)
> 
> diff --git a/common/mltools/Makefile.am b/common/mltools/Makefile.am
> index 37d10e610..ee8c319fd 100644
> --- a/common/mltools/Makefile.am
> +++ b/common/mltools/Makefile.am
> @@ -45,12 +45,12 @@ SOURCES_MLI = \
>  
>  SOURCES_ML = \
>  	getopt.ml \
> +	JSON.ml \
>  	tools_utils.ml \
>  	URI.ml \
>  	planner.ml \
>  	registry.ml \
>  	regedit.ml \
> -	JSON.ml \
>  	JSON_parser.ml \
>  	curl.ml \
>  	checksums.ml \
> diff --git a/common/mltools/tools_utils-c.c
b/common/mltools/tools_utils-c.c
> index 553aa6631..977f932d9 100644
> --- a/common/mltools/tools_utils-c.c
> +++ b/common/mltools/tools_utils-c.c
> @@ -23,6 +23,8 @@
>  #include <unistd.h>
>  #include <errno.h>
>  #include <error.h>
> +#include <time.h>
> +#include <string.h>
>  
>  #include <caml/alloc.h>
>  #include <caml/fail.h>
> @@ -41,6 +43,7 @@ extern value guestfs_int_mllib_inspect_decrypt (value gv,
value gpv, value keysv
>  extern value guestfs_int_mllib_set_echo_keys (value unitv);
>  extern value guestfs_int_mllib_set_keys_from_stdin (value unitv);
>  extern value guestfs_int_mllib_open_out_channel_from_fd (value fdv);
> +extern value guestfs_int_mllib_rfc3999_date_time_string (value unitv);
>  
>  /* Interface with the guestfish inspection and decryption code. */
>  int echo_keys = 0;
> @@ -120,3 +123,51 @@ guestfs_int_mllib_open_out_channel_from_fd (value fdv)
>  
>    CAMLreturn (caml_alloc_channel (chan));
>  }
> +
> +value
> +guestfs_int_mllib_rfc3999_date_time_string (value unitv)
> +{
> +  CAMLparam1 (unitv);
> +  char buf[64];
> +  struct timespec ts;
> +  struct tm tm;
> +  size_t ret;
> +  size_t total = 0;
> +
> +  if (clock_gettime (CLOCK_REALTIME, &ts) == -1)
> +    unix_error (errno, (char *) "clock_gettime", Val_unit);
> +
> +  if (localtime_r (&ts.tv_sec, &tm) == NULL)
> +    unix_error (errno, (char *) "localtime_r", caml_copy_int64
(ts.tv_sec));
> +
> +  /* Sadly strftime does not support nanoseconds, so what we do is:
> +   * - stringify everything before the nanoseconds
> +   * - print the nanoseconds
> +   * - stringify the rest (i.e. the timezone)
> +   * then place ':' between the hours, and the minutes of the
> +   * timezone offset.
> +   */
> +
> +  ret = strftime (buf, sizeof (buf), "%Y-%m-%dT%H:%M:%S.",
&tm);
> +  if (ret == 0)
> +    unix_error (errno, (char *) "strftime", Val_unit);
> +  total += ret;
> +
> +  ret = snprintf (buf + total, sizeof (buf) - total, "%09ld",
ts.tv_nsec);
> +  if (ret == 0)
> +    unix_error (errno, (char *) "sprintf", caml_copy_int64
(ts.tv_nsec));
> +  total += ret;
> +
> +  ret = strftime (buf + total, sizeof (buf) - total, "%z",
&tm);
> +  if (ret == 0)
> +    unix_error (errno, (char *) "strftime", Val_unit);
> +  total += ret;
> +
> +  /* Move the timezone minutes one character to the right, moving the
> +   * null character too.
> +   */
> +  memmove (buf + total - 1, buf + total - 2, 3);
> +  buf[total - 2] = ':';
> +
> +  CAMLreturn (caml_copy_string (buf));
> +}
> diff --git a/common/mltools/tools_utils.ml b/common/mltools/tools_utils.ml
> index 3c54cd4a0..1a1d11075 100644
> --- a/common/mltools/tools_utils.ml
> +++ b/common/mltools/tools_utils.ml
> @@ -33,6 +33,7 @@ external c_inspect_decrypt : Guestfs.t -> int64 ->
(string * key_store_key) list
>  external c_set_echo_keys : unit -> unit =
"guestfs_int_mllib_set_echo_keys" "noalloc"
>  external c_set_keys_from_stdin : unit -> unit =
"guestfs_int_mllib_set_keys_from_stdin" "noalloc"
>  external c_out_channel_from_fd : int -> out_channel =
"guestfs_int_mllib_open_out_channel_from_fd"
> +external c_rfc3999_date_time_string : unit -> string =
"guestfs_int_mllib_rfc3999_date_time_string"
>  
>  type machine_readable_fn = {
>    pr : 'a. ('a, unit, string, unit) format4 -> 'a;
> @@ -85,12 +86,24 @@ let ansi_magenta ?(chan = stdout) () >  let
ansi_restore ?(chan = stdout) () >    if colours () || istty chan then
output_string chan "\x1b[0m"
>  
> +let log_as_json msgtype msg > +  match machine_readable () with
> +  | None -> ()
> +  | Some { pr } ->
> +    let json = [
> +      "message", JSON.String msg;
> +      "timestamp", JSON.String (c_rfc3999_date_time_string ());
> +      "type", JSON.String msgtype;
> +    ] in
> +    pr "%s\n" (JSON.string_of_doc ~fmt:JSON.Compact json)
> +
>  (* Timestamped progress messages, used for ordinary messages when not
>   * --quiet.
>   *)
>  let start_t = Unix.gettimeofday ()
>  let message fs >    let display str > +    log_as_json
"message" str;
>      if not (quiet ()) then (
>        let t = sprintf "%.1f" (Unix.gettimeofday () -. start_t)
in
>        printf "[%6s] " t;
> @@ -105,6 +118,7 @@ let message fs >  (* Error messages etc. *)
>  let error ?(exit_code = 1) fs >    let display str > +   
log_as_json "error" str;
>      let chan = stderr in
>      ansi_red ~chan ();
>      wrap ~chan (sprintf (f_"%s: error: %s") prog str);
> @@ -123,6 +137,7 @@ let error ?(exit_code = 1) fs >  
>  let warning fs >    let display str > +    log_as_json
"warning" str;
>      let chan = stdout in
>      ansi_blue ~chan ();
>      wrap ~chan (sprintf (f_"%s: warning: %s") prog str);
> @@ -133,6 +148,7 @@ let warning fs >  
>  let info fs >    let display str > +    log_as_json "info"
str;
>      let chan = stdout in
>      ansi_magenta ~chan ();
>      wrap ~chan (sprintf (f_"%s: %s") prog str);
> diff --git a/lib/guestfs.pod b/lib/guestfs.pod
> index f11028466..3c1d635c5 100644
> --- a/lib/guestfs.pod
> +++ b/lib/guestfs.pod
> @@ -3279,6 +3279,25 @@ Some of the tools support a
I<--machine-readable> option, which is
>  generally used to make the output more machine friendly, for easier
>  parsing for example.  By default, this output goes to stdout.
>  
> +When using the I<--machine-readable> option, the progress,
> +information, warning, and error messages are also printed in JSON
> +format for easier log tracking.  Thus, it is highly recommended to
> +redirect the machine-readable output to a different stream.  The
> +format of these JSON messages is like the following (actually printed
> +within a single line, below it is indented for readability):
> +
> + {
> +   "message": "Finishing off",
> +   "timestamp": "2019-03-22T14:46:49.067294446+01:00",
> +   "type": "message"
> + }
> +
> +C<type> can be: C<message> for progress messages,
C<info> for
> +information messages, C<warning> for warning messages, and
C<error>
> +for error message.
> +C<timestamp> is the L<RFC
3999|https://www.ietf.org/rfc/rfc3339.txt>
> +timestamp of the message.
> +
>  In addition to that, a subset of these tools support an extra string
>  passed to the I<--machine-readable> option: this string specifies
>  where the machine-readable output will go.
> -- 
Yes this looks fine (and could go upstream independent of 3/4).
Such a shame that strftime can't format nanoseconds though :-(

ACK

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/

On Fri, 22 Mar 2019 16:33:43 +0100
Pino Toscano <ptoscano@redhat.com> wrote:
> When the machine readable mode is enabled, print all the messages
> (progress, info, warning, and errors) also as JSON in the machine
> readable stream: this way, users can easily parse the status of the
> OCaml tool, and report that back.
> 
> The formatting of the current date time into the RFC 3999 format is done
> in C, because of the lack of OCaml APIs for this.
> ---
>  common/mltools/Makefile.am     |  2 +-
>  common/mltools/tools_utils-c.c | 51 ++++++++++++++++++++++++++++++++++
>  common/mltools/tools_utils.ml  | 16 +++++++++++
>  lib/guestfs.pod                | 19 +++++++++++++
>  4 files changed, 87 insertions(+), 1 deletion(-)
ACK

-- 
Tomáš Golembiovský <tgolembi@redhat.com>