Libguestfs - Apr 2011 - [PATCH 0/4] Introduce "pulse mode" progress messages to the daemon.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

(Note the daemon ignores SIGPIPE.)

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
-------------- next part --------------
>From 42938f6faf9e724130be28f8e67d3c291bb81cba Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Fri, 1 Apr 2011 15:26:46 +0100
Subject: [PATCH 1/4] daemon: Reset SIGPIPE to default before running
subprocesses.

---
 daemon/guestfsd.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 1af0f7a..3632889 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -679,6 +679,7 @@ commandrvf (char **stdoutput, char **stderror, int flags,
   }
 
   if (pid == 0) {		/* Child process running the command. */
+    signal (SIGPIPE, SIG_DFL);
     close (0);
     if (flag_copy_stdin) {
       dup2 (stdin_fd[0], 0);
-- 
1.7.4.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
-------------- next part --------------
>From 6e5f64089631622167e60df25ee009ef83df5170 Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Fri, 1 Apr 2011 15:27:46 +0100
Subject: [PATCH 2/4] daemon: When running commands, restart select if we receive
a signal.

---
 daemon/guestfsd.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 3632889..1c695eb 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -768,9 +768,13 @@ commandrvf (char **stdoutput, char **stderror, int flags,
 
   quit = 0;
   while (quit < 2) {
+  again:
     rset2 = rset;
     r = select (MAX (so_fd[0], se_fd[0]) + 1, &rset2, NULL, NULL, NULL);
     if (r == -1) {
+      if (errno == EINTR)
+        goto again;
+
       perror ("select");
     quit:
       if (stdoutput) free (*stdoutput);
-- 
1.7.4.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
-------------- next part --------------
>From 40f7323134e058c0920caa18c667ea99a4c8b3e8 Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Fri, 1 Apr 2011 15:50:33 +0100
Subject: [PATCH 3/4] daemon: Introduce "pulse mode" progress events.

This introduces a new form of progress event, where we don't know how
much of the operation has taken place, but we nevertheless want to
send back some indication of activity.  Some progress bar indicators
directly support this, eg. GtkProgressBar where it is known as "pulse
mode".

A pulse mode progress message is a special backwards-compatible form
of the ordinary progress message.  No change is required in callers,
unless they want to add support for pulse mode.

The daemon sends:

 - zero or more progress messages with position = 0, total = 1
 - a single final progress message with position = total = 1

Note that the final progress message may not be sent if the call fails
and returns an error.  This is consistent with the behaviour of
ordinary progress messages.

The daemon allows two types of implementation.  Either you can just
call notify_progress (0, 1); ...; notify_progress (1, 1) as usual.

Or you can call the functions pulse_mode_start, pulse_mode_end and/or
pulse_mode_cancel (see documentation in daemon/daemon.h).  For this
second form of call, the guarantee is very weak: it *just* says the
daemon is still capable of doing something, and it doesn't imply that
if there is a subprocess that it is doing anything.  However this does
make it very easy to add pulse mode progress messages to all sorts of
existing calls that depend on long-running external commands.

To do: add a third variant that monitors a subprocess and only sends
back progress messages if it's doing something, where "doing
something" might indicate it's using CPU time or it's printing
output.
---
 daemon/configure.ac        |    4 +-
 daemon/daemon.h            |   17 +++++++
 daemon/guestfsd.c          |    1 +
 daemon/proto.c             |  115 ++++++++++++++++++++++++++++++++++++++++++++
 generator/generator_xdr.ml |   10 +++-
 src/guestfs.pod            |   14 +++++
 6 files changed, 158 insertions(+), 3 deletions(-)

diff --git a/daemon/configure.ac b/daemon/configure.ac
index 7d817d7..e5eb89b 100644
--- a/daemon/configure.ac
+++ b/daemon/configure.ac
@@ -1,5 +1,5 @@
 # libguestfs-daemon
-# Copyright (C) 2009-2010 Red Hat Inc.
+# Copyright (C) 2009-2011 Red Hat Inc.
 #
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -188,7 +188,9 @@ AC_CHECK_FUNCS([\
         posix_fallocate \
         realpath \
         removexattr \
+        setitimer \
         setxattr \
+        sigaction \
         statvfs \
         sync])
 
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 3a67758..40a087d 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -174,6 +174,23 @@ extern void reply (xdrproc_t xdrp, char *ret);
  */
 extern void notify_progress (uint64_t position, uint64_t total);
 
+/* Pulse mode progress messages.
+ *
+ * Call pulse_mode_start to start sending progress messages.
+ *
+ * Call pulse_mode_end along the ordinary exit path (ie. before a
+ * reply message is sent).
+ *
+ * Call pulse_mode_cancel along all error paths *before* any reply is
+ * sent.  pulse_mode_cancel does not modify errno, so it is safe to
+ * call it before reply_with_perror.
+ *
+ * Pulse mode and ordinary notify_progress must not be mixed.
+ */
+extern void pulse_mode_start (void);
+extern void pulse_mode_end (void);
+extern void pulse_mode_cancel (void);
+
 /* Helper for functions that need a root filesystem mounted.
  * NB. Cannot be used for FileIn functions.
  */
diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c
index 1c695eb..ac8750c 100644
--- a/daemon/guestfsd.c
+++ b/daemon/guestfsd.c
@@ -679,6 +679,7 @@ commandrvf (char **stdoutput, char **stderror, int flags,
   }
 
   if (pid == 0) {		/* Child process running the command. */
+    signal (SIGALRM, SIG_DFL);
     signal (SIGPIPE, SIG_DFL);
     close (0);
     if (flag_copy_stdin) {
diff --git a/daemon/proto.c b/daemon/proto.c
index 32137bb..6b7a187 100644
--- a/daemon/proto.c
+++ b/daemon/proto.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include <stdarg.h>
 #include <string.h>
+#include <signal.h>
 #include <inttypes.h>
 #include <unistd.h>
 #include <errno.h>
@@ -661,3 +662,117 @@ notify_progress (uint64_t position, uint64_t total)
     exit (EXIT_FAILURE);
   }
 }
+
+/* "Pulse mode" progress messages. */
+
+#if defined(HAVE_SETITIMER) && defined(HAVE_SIGACTION)
+
+static void async_safe_send_pulse (int sig);
+
+void
+pulse_mode_start (void)
+{
+  struct sigaction act;
+  struct itimerval it;
+
+  memset (&act, 0, sizeof act);
+  act.sa_handler = async_safe_send_pulse;
+  act.sa_flags = SA_RESTART;
+
+  if (sigaction (SIGALRM, &act, NULL) == -1) {
+    perror ("pulse_mode_start: sigaction");
+    return;
+  }
+
+  it.it_value.tv_sec = NOTIFICATION_INITIAL_DELAY / 1000000;
+  it.it_value.tv_usec = NOTIFICATION_INITIAL_DELAY % 1000000;
+  it.it_interval.tv_sec = NOTIFICATION_PERIOD / 1000000;
+  it.it_interval.tv_usec = NOTIFICATION_PERIOD % 1000000;
+
+  if (setitimer (ITIMER_REAL, &it, NULL) == -1)
+    perror ("pulse_mode_start: setitimer");
+}
+
+void
+pulse_mode_end (void)
+{
+  pulse_mode_cancel ();         /* Cancel the itimer. */
+
+  notify_progress (1, 1);
+}
+
+void
+pulse_mode_cancel (void)
+{
+  int err = errno;              /* Function must preserve errno. */
+  struct itimerval it;
+  struct sigaction act;
+
+  /* Setting it_value to zero cancels the itimer. */
+  it.it_value.tv_sec = 0;
+  it.it_value.tv_usec = 0;
+  it.it_interval.tv_sec = 0;
+  it.it_interval.tv_usec = 0;
+
+  if (setitimer (ITIMER_REAL, &it, NULL) == -1)
+    perror ("pulse_mode_cancel: setitimer");
+
+  memset (&act, 0, sizeof act);
+  act.sa_handler = SIG_DFL;
+
+  if (sigaction (SIGALRM, &act, NULL) == -1)
+    perror ("pulse_mode_cancel: sigaction");
+
+  errno = err;
+}
+
+/* Send a position = 0, total = 1 (pulse mode) message.  The tricky
+ * part is we have to do it without invoking any non-async-safe
+ * functions (see signal(7) for a list).  Therefore, KISS.
+ */
+static void
+async_safe_send_pulse (int sig)
+{
+  /* XDR is a RFC ... */
+  unsigned char msg[] = {
+    (GUESTFS_PROGRESS_FLAG & 0xff000000) >> 24,
+    (GUESTFS_PROGRESS_FLAG & 0x00ff0000) >> 16,
+    (GUESTFS_PROGRESS_FLAG & 0x0000ff00) >> 8,
+    GUESTFS_PROGRESS_FLAG & 0x000000ff,
+    (proc_nr & 0xff000000) >> 24,
+    (proc_nr & 0x00ff0000) >> 16,
+    (proc_nr & 0x0000ff00) >> 8,
+    proc_nr & 0x000000ff,
+    (serial & 0xff000000) >> 24,
+    (serial & 0x00ff0000) >> 16,
+    (serial & 0x0000ff00) >> 8,
+    serial & 0x000000ff,
+    /* 64 bit position = 0 */ 0, 0, 0, 0, 0, 0, 0, 0,
+    /* 64 bit total = 1 */    0, 0, 0, 0, 0, 0, 0, 1
+  };
+
+  if (xwrite (sock, msg, sizeof msg) == -1)
+    exit (EXIT_FAILURE);
+}
+
+#else /* !HAVE_SETITIMER || !HAVE_SIGACTION */
+
+void
+pulse_mode_start (void)
+{
+  /* empty */
+}
+
+void
+pulse_mode_end (void)
+{
+  /* empty */
+}
+
+void
+pulse_mode_cancel (void)
+{
+  /* empty */
+}
+
+#endif /* !HAVE_SETITIMER || !HAVE_SIGACTION */
diff --git a/generator/generator_xdr.ml b/generator/generator_xdr.ml
index 5714c80..07f3ff9 100644
--- a/generator/generator_xdr.ml
+++ b/generator/generator_xdr.ml
@@ -1,5 +1,5 @@
 (* libguestfs
- * Copyright (C) 2009-2010 Red Hat Inc.
+ * Copyright (C) 2009-2011 Red Hat Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -214,8 +214,14 @@ struct guestfs_chunk {
  * 'position' and 'total' have undefined units; however they
may
  * have meaning for some calls.
  *
- * NB. guestfs___recv_from_daemon assumes the XDR-encoded
+ * Notes:
+ *
+ * (1) guestfs___recv_from_daemon assumes the XDR-encoded
  * structure is 24 bytes long.
+ *
+ * (2) daemon/proto.c:async_safe_send_pulse assumes the progress
+ * message is laid out precisely in this way.  So if you change
+ * this then you'd better change that function as well.
  */
 struct guestfs_progress {
   guestfs_procedure proc;            /* @0:  GUESTFS_PROC_x */
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 91c6b33..eecf96d 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -1786,6 +1786,20 @@ This is to simplify caller code, so callers can easily
set the
 progress indicator to "100%" at the end of the operation, without
 requiring special code to detect this case.
 
+=item *
+
+For some calls we are unable to estimate the progress of the call, but
+we can still generate progress messages to indicate activity.  This is
+known as "pulse mode", and is directly supported by certain progress
+bar implementations (eg. GtkProgressBar).
+
+For these calls, zero or more progress messages are generated with
+C<position = 0> and C<total = 1>, followed by a final message with
+C<position = total = 1>.
+
+As noted above, if the call fails with an error then the final message
+may not be generated.
+
 =back
 
 The callback also receives the procedure number (C<proc_nr>) and
-- 
1.7.4.1

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org
-------------- next part --------------
>From c1f7c0d03843c8cb41a012c691be6137002f5500 Mon Sep 17 00:00:00 2001
From: Richard W.M. Jones <rjones at redhat.com>
Date: Fri, 1 Apr 2011 16:03:14 +0100
Subject: [PATCH 4/4] du: Add pulse mode progress messages.

---
 daemon/du.c                    |    8 +++++++-
 generator/generator_actions.ml |    2 +-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/daemon/du.c b/daemon/du.c
index 7cf2f4a..1c44b3b 100644
--- a/daemon/du.c
+++ b/daemon/du.c
@@ -1,5 +1,5 @@
 /* libguestfs - the guestfsd daemon
- * Copyright (C) 2009 Red Hat Inc.
+ * Copyright (C) 2009-2011 Red Hat Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -43,9 +43,12 @@ do_du (const char *path)
     return -1;
   }
 
+  pulse_mode_start ();
+
   r = command (&out, &err, "du", "-s", buf, NULL);
   free (buf);
   if (r == -1) {
+    pulse_mode_cancel ();
     reply_with_error ("%s: %s", path, err);
     free (out);
     free (err);
@@ -55,6 +58,7 @@ do_du (const char *path)
   free (err);
 
   if (sscanf (out, "%"SCNi64, &rv) != 1) {
+    pulse_mode_cancel ();
     reply_with_error ("%s: could not read output: %s", path, out);
     free (out);
     return -1;
@@ -62,5 +66,7 @@ do_du (const char *path)
 
   free (out);
 
+  pulse_mode_end ();
+
   return rv;
 }
diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
index a324d99..a10b7f7 100644
--- a/generator/generator_actions.ml
+++ b/generator/generator_actions.ml
@@ -3577,7 +3577,7 @@ This command is mostly useful for interactive sessions. 
It
 is I<not> intended that you try to parse the output string.
 Use C<guestfs_statvfs> from programs.");
 
-  ("du", (RInt64 "sizekb", [Pathname "path"],
[]), 127, [],
+  ("du", (RInt64 "sizekb", [Pathname "path"],
[]), 127, [Progress],
    [InitISOFS, Always, TestOutputInt (
       [["du"; "/directory"]], 2 (* ISO fs blocksize is 2K
*))],
    "estimate file space usage",
-- 
1.7.4.1