Libguestfs - Jul 2011 - [PATCH 0/8] Implement user cancellation

This patch series implements user cancellation.  What this means is
that the "user" (or any library caller) can cancel certain
long-running operations.  Currently it is only possible to cancel
upload and download operations (ie. anything in the generator which
uses FileIn and FileOut).

The mechanism in the protocol to implement cancellation already
exists, and it is already used to cancel transfers where there is an
error.  For example, if the library gets an error while reading a
local file during an upload, it already cancels the transfer.

Therefore the only thing left to do is to expose cancellation to
library callers.  This is done through the new API call:

  void guestfs_user_cancel (guestfs_h *g);

This call is signal safe and thread safe.  It is, in fact, designed to
be called from a SIGINT signal handler (to handle ^C being pressed by
the user during a transfer).  You could also wire it up to a cancel
button called from another thread, or use it in other ways.

There are three main complexities to this patch series:

(1) We need to implement process groups (see patch 2/8).  This is
because otherwise the terminal sends ^C to the qemu process (because
it is in the same process group as the parent), killing it.  In order
to catch ^C in guestfish, we need guestfish and the subprocesses to be
in different process groups.

(2) The language bindings need to be signal safe.  This turns out to
be simple for OCaml, Perl and Ruby.  However Python doesn't have any
clear way to make functions signal safe, and so I did not implement a
Python binding for now.

(3) There is a multithreaded test program (capitests/test-user-cancel.c)
which performs an upload and a download and attempts to cancel it at
the same time.

Rich.

-- 
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

-- 
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 4b53a3737b76cad7aca407a0d9e37fed1d42ef69 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Fri, 15 Jul 2011 10:42:56 +0100
Subject: [PATCH 1/8] guestfs.h: Add missing extern keyword before event
 functions.

---
 generator/generator_c.ml |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/generator/generator_c.ml b/generator/generator_c.ml
index 2e9607b..4537475 100644
--- a/generator/generator_c.ml
+++ b/generator/generator_c.ml
@@ -436,13 +436,13 @@ typedef void (*guestfs_event_callback) (
 #endif
 
 #define LIBGUESTFS_HAVE_SET_EVENT_CALLBACK 1
-int guestfs_set_event_callback (guestfs_h *g,
-                                guestfs_event_callback cb,
-                                uint64_t event_bitmask,
-                                int flags,
-                                void *opaque);
+extern int guestfs_set_event_callback (guestfs_h *g,
+                                       guestfs_event_callback cb,
+                                       uint64_t event_bitmask,
+                                       int flags,
+                                       void *opaque);
 #define LIBGUESTFS_HAVE_DELETE_EVENT_CALLBACK 1
-void guestfs_delete_event_callback (guestfs_h *g, int event_handle);
+extern void guestfs_delete_event_callback (guestfs_h *g, int event_handle);
 
 /* Old-style event handling. */
 #ifndef GUESTFS_TYPEDEF_LOG_MESSAGE_CB
-- 
1.7.5.2

-- 
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 f173543fd207bdc254a5eb75180d82ef25eacae9 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Fri, 15 Jul 2011 11:38:21 +0100
Subject: [PATCH 2/8] New APIs: set-pgroup, get-pgroup

If the pgroup flag is set in the handle, then the qemu and recovery
subprocesses are placed in separate process groups.  The default is
false.

The purpose for setting up a process group is that ^C will not be
passed from the main process down to these processes (killing them).
This allows ^C and other keyboard events to be caught and handled in
the main process.
---
 generator/generator_actions.ml |   19 +++++++++++++++++++
 src/guestfs-internal.h         |    2 ++
 src/guestfs.c                  |   13 +++++++++++++
 src/launch.c                   |   17 +++++++++++------
 4 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/generator/generator_actions.ml b/generator/generator_actions.ml
index 1ccacdb..87c934a 100644
--- a/generator/generator_actions.ml
+++ b/generator/generator_actions.ml
@@ -1526,6 +1526,25 @@ advice before using trademarks in applications.
 
 =back");
 
+  ("set_pgroup", (RErr, [Bool "pgroup"], []), -1,
[FishAlias "pgroup"],
+   [],
+   "set process group flag",
+   "\
+If C<pgroup> is true, child processes are placed into
+their own process group.
+
+The practical upshot of this is that signals like C<SIGINT> (from
+users pressing C<^C>) won't be received by the child process.
+
+The default for this flag is false, because usually you want
+C<^C> to kill the subprocess.");
+
+  ("get_pgroup", (RBool "pgroup", [], []), -1, [],
+   [],
+   "get process group flag",
+   "\
+This returns the process group flag.");
+
 ]
 
 (* daemon_functions are any functions which cause some action
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 96f8152..e2ffdf3 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -170,6 +170,8 @@ struct guestfs_h
 
   int selinux;                  /* selinux enabled? */
 
+  int pgroup;                   /* Create process group for children? */
+
   char *last_error;
   int last_errnum;              /* errno, or 0 if there was no errno */
 
diff --git a/src/guestfs.c b/src/guestfs.c
index b02bdb9..e2b7159 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -801,6 +801,19 @@ guestfs__get_attach_method (guestfs_h *g)
   return ret;
 }
 
+int
+guestfs__set_pgroup (guestfs_h *g, int v)
+{
+  g->pgroup = !!v;
+  return 0;
+}
+
+int
+guestfs__get_pgroup (guestfs_h *g)
+{
+  return g->pgroup;
+}
+
 /* Note the private data area is allocated lazily, since the vast
  * majority of callers will never use it.  This means g->pda is
  * likely to be NULL.
diff --git a/src/launch.c b/src/launch.c
index 0b15ce9..1a47363 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -643,12 +643,9 @@ launch_appliance (guestfs_h *g)
       close (rfd[1]);
     }
 
-#if 0
-    /* Set up a new process group, so we can signal this process
-     * and all subprocesses (eg. if qemu is really a shell script).
-     */
-    setpgid (0, 0);
-#endif
+    /* Put qemu in a new process group. */
+    if (g->pgroup)
+      setpgid (0, 0);
 
     setenv ("LC_ALL", "C", 1);
 
@@ -677,6 +674,14 @@ launch_appliance (guestfs_h *g)
       pid_t qemu_pid = g->pid;
       pid_t parent_pid = getppid ();
 
+      /* It would be nice to be able to put this in the same process
+       * group as qemu (ie. setpgid (0, qemu_pid)).  However this is
+       * not possible because we don't have any guarantee here that
+       * the qemu process has started yet.
+       */
+      if (g->pgroup)
+        setpgid (0, 0);
+
       /* Writing to argv is hideously complicated and error prone.  See:
        *
http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc/ps_status.c?rev=1.33.2.1;content-type=text%2Fplain
        */
-- 
1.7.5.2

-- 
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 9ec9c97ce2ba46e56b304d3a367c86adc703160d Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Fri, 15 Jul 2011 10:43:36 +0100
Subject: [PATCH 3/8] Add user cancellation to the C API.

This allows long transfers (FileIn and FileOut operations) to be
cancelled by calling the signal and thread safe guestfs_user_cancel
function.

Most of this commit consists of a multithreaded program that tests
user cancellation of uploads and downloads.
---
 .gitignore                   |    1 +
 capitests/Makefile.am        |   10 ++
 capitests/test-user-cancel.c |  327 ++++++++++++++++++++++++++++++++++++++++++
 generator/generator_c.ml     |    5 +
 src/guestfs-internal.h       |    5 +
 src/guestfs.c                |    6 +
 src/guestfs.pod              |   39 +++++
 src/proto.c                  |   27 +++-
 8 files changed, 413 insertions(+), 7 deletions(-)
 create mode 100644 capitests/test-user-cancel.c

diff --git a/.gitignore b/.gitignore
index db63861..19d971b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -20,6 +20,7 @@ capitests/test-debug-to-file
 capitests/test-just-header
 capitests/test-last-errno
 capitests/test-private-data
+capitests/test-user-cancel
 capitests/test*.img
 capitests/tests
 capitests/tests.c
diff --git a/capitests/Makefile.am b/capitests/Makefile.am
index 487a33f..7e35872 100644
--- a/capitests/Makefile.am
+++ b/capitests/Makefile.am
@@ -32,6 +32,7 @@ check_PROGRAMS = \
 	test-add-drive-opts \
 	test-last-errno \
 	test-private-data \
+	test-user-cancel \
 	test-debug-to-file
 
 TESTS = \
@@ -42,6 +43,7 @@ TESTS = \
 	test-add-drive-opts \
 	test-last-errno \
 	test-private-data \
+	test-user-cancel \
 	test-debug-to-file
 
 # The API behind this test is not baked yet.
@@ -114,6 +116,14 @@ test_private_data_CFLAGS = \
 test_private_data_LDADD = \
 	$(top_builddir)/src/libguestfs.la
 
+test_user_cancel_SOURCES = test-user-cancel.c
+test_user_cancel_CFLAGS = \
+	-pthread \
+	-I$(top_srcdir)/src -I$(top_builddir)/src \
+	$(WARN_CFLAGS) $(WERROR_CFLAGS)
+test_user_cancel_LDADD = \
+	$(top_builddir)/src/libguestfs.la
+
 test_debug_to_file_SOURCES = test-debug-to-file.c
 test_debug_to_file_CFLAGS = \
 	-I$(top_srcdir)/src -I$(top_builddir)/src \
diff --git a/capitests/test-user-cancel.c b/capitests/test-user-cancel.c
new file mode 100644
index 0000000..429998e
--- /dev/null
+++ b/capitests/test-user-cancel.c
@@ -0,0 +1,327 @@
+/* libguestfs
+ * Copyright (C) 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
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+/* Test user cancellation.
+ *
+ * We perform the test using two threads.  The main thread issues
+ * guestfs commands to download and upload large files.  Uploads and
+ * downloads are done to/from a pipe which is connected back to the
+ * current process.  The second test thread sits on the other end of
+ * the pipe, feeding or consuming data slowly, and injecting the user
+ * cancel events at a particular place in the transfer.
+ *
+ * It is important to test both download and upload separately, since
+ * these exercise different code paths in the library.  However this
+ * adds complexity here because these tests are symmetric-but-opposite
+ * cases.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <sys/time.h>
+
+#include <pthread.h>
+
+#include "guestfs.h"
+
+static const char *filename = "test.img";
+static const off_t filesize = 1024*1024*1024;
+
+static void remove_test_img (void);
+static void *start_test_thread (void *);
+
+struct test_thread_data {
+  guestfs_h *g;                /* handle */
+  int direction;               /* direction of transfer */
+#define DIRECTION_UP 1         /* upload (test thread is writing) */
+#define DIRECTION_DOWN 2       /* download (test thread is reading) */
+  int fd;                      /* pipe to read/write */
+  off_t cancel_posn;           /* position at which to cancel */
+  off_t transfer_size;         /* how much data thread wrote/read */
+};
+
+int
+main (int argc, char *argv[])
+{
+  guestfs_h *g;
+  int fd;
+  char c = 0;
+  pthread_t test_thread;
+  struct test_thread_data data;
+  int fds[2], r, op_error, op_errno, errors = 0;
+  char dev_fd[64];
+
+  srandom (time (NULL));
+
+  g = guestfs_create ();
+  if (g == NULL) {
+    fprintf (stderr, "failed to create handle\n");
+    exit (EXIT_FAILURE);
+  }
+
+  /* Create a test image and test data. */
+  fd = open (filename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY, 0666);
+  if (fd == -1) {
+    perror (filename);
+    exit (EXIT_FAILURE);
+  }
+
+  atexit (remove_test_img);
+
+  if (lseek (fd, filesize - 1, SEEK_SET) == (off_t) -1) {
+    perror ("lseek");
+    close (fd);
+    exit (EXIT_FAILURE);
+  }
+
+  if (write (fd, &c, 1) != 1) {
+    perror ("write");
+    close (fd);
+    exit (EXIT_FAILURE);
+  }
+
+  if (close (fd) == -1) {
+    perror ("test.img");
+    exit (EXIT_FAILURE);
+  }
+
+  if (guestfs_add_drive_opts (g, filename,
+                              GUESTFS_ADD_DRIVE_OPTS_FORMAT, "raw",
+                              -1) == -1)
+    exit (EXIT_FAILURE);
+
+  if (guestfs_launch (g) == -1)
+    exit (EXIT_FAILURE);
+
+  if (guestfs_part_disk (g, "/dev/sda", "mbr") == -1)
+    exit (EXIT_FAILURE);
+
+  if (guestfs_mkfs (g, "ext2", "/dev/sda1") == -1)
+    exit (EXIT_FAILURE);
+
+  if (guestfs_mount_options (g, "", "/dev/sda1",
"/") == -1)
+    exit (EXIT_FAILURE);
+
+  /*----- Upload cancellation test -----*/
+
+  data.g = g;
+  data.direction = DIRECTION_UP;
+
+  if (pipe (fds) == -1) {
+    perror ("pipe");
+    exit (EXIT_FAILURE);
+  }
+
+  data.fd = fds[1];
+  snprintf (dev_fd, sizeof dev_fd, "/dev/fd/%d", fds[0]);
+
+  data.cancel_posn = random () % (filesize/4);
+
+  /* Create the test thread. */
+  r = pthread_create (&test_thread, NULL, start_test_thread, &data);
+  if (r != 0) {
+    fprintf (stderr, "pthread_create: %s", strerror (r));
+    exit (EXIT_FAILURE);
+  }
+
+  /* Do the upload. */
+  op_error = guestfs_upload (g, dev_fd, "/upload");
+  op_errno = guestfs_last_errno (g);
+
+  /* Kill the test thread and clean up. */
+  r = pthread_cancel (test_thread);
+  if (r != 0) {
+    fprintf (stderr, "pthread_cancel: %s", strerror (r));
+    exit (EXIT_FAILURE);
+  }
+  r = pthread_join (test_thread, NULL);
+  if (r != 0) {
+    fprintf (stderr, "pthread_join: %s", strerror (r));
+    exit (EXIT_FAILURE);
+  }
+
+  close (fds[0]);
+  close (fds[1]);
+
+  /* We expect to get an error, with errno == EINTR. */
+  if (op_error == -1 && op_errno == EINTR)
+    printf ("test-user-cancel: upload cancellation test passed
(%ld/%ld)\n",
+            (long) data.cancel_posn, (long) data.transfer_size);
+  else {
+    fprintf (stderr, "test-user-cancel: upload cancellation test
FAILED\n");
+    fprintf (stderr, "cancel_posn %ld, upload returned %d, errno = %d
(%s)\n",
+             (long) data.cancel_posn, op_error, op_errno, strerror (op_errno));
+    errors++;
+  }
+
+  if (guestfs_rm (g, "/upload") == -1)
+    exit (EXIT_FAILURE);
+
+  /*----- Download cancellation test -----*/
+
+  if (guestfs_touch (g, "/download") == -1)
+    exit (EXIT_FAILURE);
+
+  if (guestfs_truncate_size (g, "/download", filesize/4) == -1)
+    exit (EXIT_FAILURE);
+
+  data.g = g;
+  data.direction = DIRECTION_DOWN;
+
+  if (pipe (fds) == -1) {
+    perror ("pipe");
+    exit (EXIT_FAILURE);
+  }
+
+  data.fd = fds[0];
+  snprintf (dev_fd, sizeof dev_fd, "/dev/fd/%d", fds[1]);
+
+  data.cancel_posn = random () % (filesize/4);
+
+  /* Create the test thread. */
+  r = pthread_create (&test_thread, NULL, start_test_thread, &data);
+  if (r != 0) {
+    fprintf (stderr, "pthread_create: %s", strerror (r));
+    exit (EXIT_FAILURE);
+  }
+
+  /* Do the download. */
+  op_error = guestfs_download (g, "/download", dev_fd);
+  op_errno = guestfs_last_errno (g);
+
+  /* Kill the test thread and clean up. */
+  r = pthread_cancel (test_thread);
+  if (r != 0) {
+    fprintf (stderr, "pthread_cancel: %s", strerror (r));
+    exit (EXIT_FAILURE);
+  }
+  r = pthread_join (test_thread, NULL);
+  if (r != 0) {
+    fprintf (stderr, "pthread_join: %s", strerror (r));
+    exit (EXIT_FAILURE);
+  }
+
+  close (fds[0]);
+  close (fds[1]);
+
+  /* We expect to get an error, with errno == EINTR. */
+  if (op_error == -1 && op_errno == EINTR)
+    printf ("test-user-cancel: download cancellation test passed
(%ld/%ld)\n",
+            (long) data.cancel_posn, (long) data.transfer_size);
+  else {
+    fprintf (stderr, "test-user-cancel: download cancellation test
FAILED\n");
+    fprintf (stderr, "cancel_posn %ld, upload returned %d, errno = %d
(%s)\n",
+             (long) data.cancel_posn, op_error, op_errno, strerror (op_errno));
+    errors++;
+  }
+
+  exit (errors == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
+}
+
+static void
+remove_test_img (void)
+{
+  unlink (filename);
+}
+
+static char buffer[BUFSIZ];
+
+#ifndef MIN
+#define MIN(a,b) ((a)<(b)?(a):(b))
+#endif
+
+static void *
+start_test_thread (void *datav)
+{
+  struct test_thread_data *data = datav;
+  ssize_t r;
+  size_t n;
+
+  data->transfer_size = 0;
+
+  if (data->direction == DIRECTION_UP) { /* thread is writing */
+    /* Feed data in, up to the cancellation point. */
+    while (data->transfer_size < data->cancel_posn) {
+      n = MIN (sizeof buffer,
+               (size_t) (data->cancel_posn - data->transfer_size));
+      r = write (data->fd, buffer, n);
+      if (r == -1) {
+        perror ("test thread: write to pipe before user cancel");
+        exit (EXIT_FAILURE);
+      }
+      data->transfer_size += r;
+    }
+
+    /* Do user cancellation. */
+    guestfs_user_cancel (data->g);
+
+    /* Keep feeding data after the cancellation point for as long as
+     * the main thread wants it.
+     */
+    while (1) {
+      r = write (data->fd, buffer, sizeof buffer);
+      if (r == -1) {
+        perror ("test thread: write to pipe after user cancel");
+        exit (EXIT_FAILURE);
+      }
+      data->transfer_size += r;
+    }
+  } else {                      /* thread is reading */
+    /* Sink data, up to the cancellation point. */
+    while (data->transfer_size < data->cancel_posn) {
+      n = MIN (sizeof buffer,
+               (size_t) (data->cancel_posn - data->transfer_size));
+      r = read (data->fd, buffer, n);
+      if (r == -1) {
+        perror ("test thread: read from pipe before user cancel");
+        exit (EXIT_FAILURE);
+      }
+      if (r == 0) {
+        perror ("test thread: unexpected end of file before user
cancel");
+        exit (EXIT_FAILURE);
+      }
+      data->transfer_size += r;
+    }
+
+    /* Do user cancellation. */
+    guestfs_user_cancel (data->g);
+
+    /* Keep sinking data as long as the main thread is writing. */
+    while (1) {
+      r = read (data->fd, buffer, sizeof buffer);
+      if (r == -1) {
+        perror ("test thread: read from pipe after user cancel");
+        exit (EXIT_FAILURE);
+      }
+      if (r == 0)
+        break;
+      data->transfer_size += r;
+    }
+
+    while (1)
+      pause ();
+  }
+
+  return NULL;
+}
diff --git a/generator/generator_c.ml b/generator/generator_c.ml
index 4537475..fa9c0ff 100644
--- a/generator/generator_c.ml
+++ b/generator/generator_c.ml
@@ -483,6 +483,10 @@ extern void guestfs_set_close_callback (guestfs_h *g,
guestfs_close_cb cb, void
 extern void guestfs_set_progress_callback (guestfs_h *g, guestfs_progress_cb
cb, void *opaque)
   GUESTFS_DEPRECATED_BY(\"set_event_callback\");
 
+/* User cancellation. */
+#define LIBGUESTFS_HAVE_USER_CANCEL 1
+extern void guestfs_user_cancel (guestfs_h *g);
+
 /* Private data area. */
 #define LIBGUESTFS_HAVE_SET_PRIVATE 1
 extern void guestfs_set_private (guestfs_h *g, const char *key, void *data);
@@ -1488,6 +1492,7 @@ and generate_linker_script ()     
"guestfs_set_private";
     "guestfs_set_progress_callback";
     "guestfs_set_subprocess_quit_callback";
+    "guestfs_user_cancel";
 
     (* Unofficial parts of the API: the bindings code use these
      * functions, so it is useful to export them.
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index e2ffdf3..99a0404 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -200,6 +200,11 @@ struct guestfs_h
   FILE *trace_fp;
   char *trace_buf;
   size_t trace_len;
+
+  /* User cancelled transfer.  Not signal-atomic, but it doesn't
+   * matter for this case because we only care if it is != 0.
+   */
+  int user_cancel;
 };
 
 /* Per-filesystem data stored for inspect_os. */
diff --git a/src/guestfs.c b/src/guestfs.c
index e2b7159..1fa3c0a 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -578,6 +578,12 @@ guestfs_get_error_handler (guestfs_h *g, void **data_rtn)
   return g->error_cb;
 }
 
+void
+guestfs_user_cancel (guestfs_h *g)
+{
+  g->user_cancel = 1;
+}
+
 int
 guestfs__set_verbose (guestfs_h *g, int v)
 {
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 842b0e4..341321f 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -1949,6 +1949,45 @@ this example, messages are directed to syslog:
      syslog (priority, "event 0x%lx: %s", event, buf);
  }
 
+=head1 CANCELLING LONG TRANSFERS
+
+Some operations can be cancelled by the caller while they are in
+progress.  Currently only operations that involve uploading or
+downloading data can be cancelled (technically: operations that have
+C<FileIn> or C<FileOut> parameters in the generator).
+
+=head2 guestfs_user_cancel
+
+ void guestfs_user_cancel (guestfs_h *g);
+
+C<guestfs_user_cancel> cancels the current upload or download
+operation.
+
+Unlike most other libguestfs calls, this function is signal safe and
+thread safe.  You can call it from a signal handler or from another
+thread, without needing to do any locking.
+
+The transfer that was in progress (if there is one) will stop shortly
+afterwards, and will return an error.  The errno (see
+L</guestfs_last_errno>) is set to C<EINTR>, so you can test for
this
+to find out if the operation was cancelled or failed because of
+another error.
+
+No cleanup is performed: for example, if a file was being uploaded
+then after cancellation there may be a partially uploaded file.  It is
+the caller's responsibility to clean up if necessary.
+
+There are two common places that you might call C<guestfs_user_cancel>.
+
+In an interactive text-based program, you might call it from a
+C<SIGINT> signal handler so that pressing C<^C> cancels the current
+operation.  (You also need to call L</guestfs_set_pgroup> so that
+child processes don't receive the C<^C> signal).
+
+In a graphical program, when the main thread is displaying a progress
+bar with a cancel button, wire up the cancel button to call this
+function.
+
 =head1 PRIVATE DATA AREA
 
 You can attach named pieces of private data to the libguestfs handle,
diff --git a/src/proto.c b/src/proto.c
index d8b1048..be7fbdc 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -872,7 +872,6 @@ guestfs___send (guestfs_h *g, int proc_nr,
   return -1;
 }
 
-static int cancel = 0; /* XXX Implement file cancellation. */
 static int send_file_chunk (guestfs_h *g, int cancel, const char *buf, size_t
len);
 static int send_file_data (guestfs_h *g, const char *buf, size_t len);
 static int send_file_cancellation (guestfs_h *g);
@@ -888,7 +887,9 @@ int
 guestfs___send_file (guestfs_h *g, const char *filename)
 {
   char buf[GUESTFS_MAX_CHUNK_SIZE];
-  int fd, r, err;
+  int fd, r = 0, err;
+
+  g->user_cancel = 0;
 
   fd = open (filename, O_RDONLY);
   if (fd == -1) {
@@ -898,7 +899,7 @@ guestfs___send_file (guestfs_h *g, const char *filename)
   }
 
   /* Send file in chunked encoding. */
-  while (!cancel) {
+  while (!g->user_cancel) {
     r = read (fd, buf, sizeof buf);
     if (r == -1 && (errno == EINTR || errno == EAGAIN))
       continue;
@@ -911,13 +912,15 @@ guestfs___send_file (guestfs_h *g, const char *filename)
     }
   }
 
-  if (cancel) {			/* cancel from either end */
+  if (r == -1) {
+    perrorf (g, "read: %s", filename);
     send_file_cancellation (g);
     return -1;
   }
 
-  if (r == -1) {
-    perrorf (g, "read: %s", filename);
+  if (g->user_cancel) {
+    error (g, _("operation cancelled by user"));
+    g->last_errnum = EINTR;
     send_file_cancellation (g);
     return -1;
   }
@@ -1116,6 +1119,8 @@ guestfs___recv_file (guestfs_h *g, const char *filename)
   void *buf;
   int fd, r;
 
+  g->user_cancel = 0;
+
   fd = open (filename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY, 0666);
   if (fd == -1) {
     perrorf (g, "open: %s", filename);
@@ -1130,6 +1135,9 @@ guestfs___recv_file (guestfs_h *g, const char *filename)
       goto cancel;
     }
     free (buf);
+
+    if (g->user_cancel)
+      goto cancel;
   }
 
   if (r == -1) {
@@ -1205,7 +1213,12 @@ receive_file_data (guestfs_h *g, void **buf_r)
   free (buf);
 
   if (chunk.cancel) {
-    error (g, _("file receive cancelled by daemon"));
+    if (g->user_cancel) {
+      error (g, _("operation cancelled by user"));
+      g->last_errnum = EINTR;
+    }
+    else
+      error (g, _("file receive cancelled by daemon"));
     free (chunk.data.data_val);
     return -1;
   }
-- 
1.7.5.2

-- 
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 e187aa8431ccf0436d9aee03ee1e80b79f148217 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Fri, 15 Jul 2011 11:36:23 +0100
Subject: [PATCH 4/8] fish: Add is_interactive flag.

Decide early (before launch) if this is going to be an interactive
session, and set the is_interactive flag.
---
 fish/fish.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/fish/fish.c b/fish/fish.c
index 4e45cea..750f50d 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -89,6 +89,7 @@ int inspector = 0;
 int utf8_mode = 0;
 int have_terminfo = 0;
 int progress_bars = 0;
+int is_interactive = 0;
 
 static void __attribute__((noreturn))
 usage (int status)
@@ -385,6 +386,12 @@ main (int argc, char *argv[])
     }
   }
 
+  /* Decide here if this will be an interactive session.  We have to
+   * do this as soon as possible after processing the command line
+   * args.
+   */
+  is_interactive = !file && isatty (0);
+
   /* Old-style -i syntax?  Since -a/-d/-N and -i was disallowed
    * previously, if we have -i without any drives but with something
    * on the command line, it must be old-style syntax.
@@ -487,7 +494,7 @@ main (int argc, char *argv[])
   progress_bars      override_progress_bars >= 0
     ? override_progress_bars
-    : (optind >= argc && isatty (0));
+    : (optind >= argc && is_interactive);
 
   if (progress_bars)
     guestfs_set_event_callback (g, progress_callback,
@@ -495,7 +502,7 @@ main (int argc, char *argv[])
 
   /* Interactive, shell script, or command(s) on the command line? */
   if (optind >= argc) {
-    if (isatty (0))
+    if (is_interactive)
       interactive ();
     else
       shell_script ();
-- 
1.7.5.2

-- 
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 50c54d2814b13d7d51f659600e93565dc6921b28 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Fri, 15 Jul 2011 11:01:23 +0100
Subject: [PATCH 5/8] fish: Register ^C handler to cancel long transfers.

---
 fish/fish.c   |   22 +++++++++++++++++++++-
 fish/reopen.c |    4 ++++
 2 files changed, 25 insertions(+), 1 deletions(-)

diff --git a/fish/fish.c b/fish/fish.c
index 750f50d..2dbcdf0 100644
--- a/fish/fish.c
+++ b/fish/fish.c
@@ -53,6 +53,7 @@ struct parsed_command {
   char *argv[64];
 };
 
+static void user_cancel (int);
 static void set_up_terminal (void);
 static void prepare_drives (struct drv *drv);
 static int launch (void);
@@ -72,7 +73,7 @@ static void add_history_line (const char *);
 static int override_progress_bars = -1;
 
 /* Currently open libguestfs handle. */
-guestfs_h *g;
+guestfs_h *g = NULL;
 
 int read_only = 0;
 int live = 0;
@@ -392,6 +393,18 @@ main (int argc, char *argv[])
    */
   is_interactive = !file && isatty (0);
 
+  /* Register a ^C handler.  We have to do this before launch could
+   * possibly be called below.
+   */
+  if (is_interactive) {
+    memset (&sa, 0, sizeof sa);
+    sa.sa_handler = user_cancel;
+    sa.sa_flags = SA_RESTART;
+    sigaction (SIGINT, &sa, NULL);
+
+    guestfs_set_pgroup (g, 1);
+  }
+
   /* Old-style -i syntax?  Since -a/-d/-N and -i was disallowed
    * previously, if we have -i without any drives but with something
    * on the command line, it must be old-style syntax.
@@ -515,6 +528,13 @@ main (int argc, char *argv[])
   exit (EXIT_SUCCESS);
 }
 
+static void
+user_cancel (int sig)
+{
+  if (g)
+    guestfs_user_cancel (g);
+}
+
 /* The <term.h> header file which defines this has "issues". */
 extern int tgetent (char *, const char *);
 
diff --git a/fish/reopen.c b/fish/reopen.c
index 67a845c..959cd2c 100644
--- a/fish/reopen.c
+++ b/fish/reopen.c
@@ -66,6 +66,10 @@ run_reopen (const char *cmd, size_t argc, char *argv[])
   if (p)
     guestfs_set_path (g2, p);
 
+  r = guestfs_get_pgroup (g);
+  if (r >= 0)
+    guestfs_set_pgroup (g2, r);
+
   if (progress_bars)
     guestfs_set_event_callback (g2, progress_callback,
                                 GUESTFS_EVENT_PROGRESS, 0, NULL);
-- 
1.7.5.2

-- 
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 60dd9494f0890dcc6c9a1cce311edc92cb992290 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Fri, 15 Jul 2011 14:33:29 +0100
Subject: [PATCH 6/8] ocaml: Add binding for guestfs_user_cancel.

---
 generator/generator_ocaml.ml |    8 ++++++++
 ocaml/guestfs_c.c            |   11 +++++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/generator/generator_ocaml.ml b/generator/generator_ocaml.ml
index aa1adec..aafc6cb 100644
--- a/generator/generator_ocaml.ml
+++ b/generator/generator_ocaml.ml
@@ -94,6 +94,10 @@ val delete_event_callback : t -> event_handle -> unit
 (** [delete_event_callback g eh] removes a previously registered
     event callback.  See {!set_event_callback}. *)
 
+val user_cancel : t -> unit
+(** Cancel current transfer.  This is safe to call from OCaml signal
+    handlers and threads. *)
+
 ";
   generate_ocaml_structure_decls ();
 
@@ -129,6 +133,7 @@ class guestfs : unit -> object
   method close : unit -> unit
   method set_event_callback : event_callback -> event list ->
event_handle
   method delete_event_callback : event_handle -> unit
+  method user_cancel : unit -> unit
   method ocaml_handle : t
 ";
 
@@ -188,6 +193,8 @@ external set_event_callback : t -> event_callback ->
event list -> event_handle
 external delete_event_callback : t -> event_handle -> unit
   = \"ocaml_guestfs_delete_event_callback\"
 
+external user_cancel : t -> unit = \"ocaml_guestfs_user_cancel\"
\"noalloc\"
+
 (* Give the exceptions names, so they can be raised from the C code. *)
 let ()    Callback.register_exception \"ocaml_guestfs_error\" (Error
\"\");
@@ -211,6 +218,7 @@ class guestfs ()      method close () = close g
     method set_event_callback = set_event_callback g
     method delete_event_callback = delete_event_callback g
+    method user_cancel () = user_cancel g
     method ocaml_handle = g
 ";
 
diff --git a/ocaml/guestfs_c.c b/ocaml/guestfs_c.c
index a1386ec..45b8eae 100644
--- a/ocaml/guestfs_c.c
+++ b/ocaml/guestfs_c.c
@@ -52,6 +52,7 @@ CAMLprim value ocaml_guestfs_create (void);
 CAMLprim value ocaml_guestfs_close (value gv);
 CAMLprim value ocaml_guestfs_set_event_callback (value gv, value closure, value
events);
 CAMLprim value ocaml_guestfs_delete_event_callback (value gv, value eh);
+value ocaml_guestfs_user_cancel (value gv);
 
 /* Allocate handles and deal with finalization. */
 static void
@@ -372,3 +373,13 @@ event_callback_wrapper (guestfs_h *g,
 
   CAMLreturn0;
 }
+
+/* NB: This is and must remain a "noalloc" function. */
+value
+ocaml_guestfs_user_cancel (value gv)
+{
+  guestfs_h *g = Guestfs_val (gv);
+  if (g)
+    guestfs_user_cancel (g);
+  return Val_unit;
+}
-- 
1.7.5.2

-- 
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 f4dadd0fcfe41b9cc6fcd6097c4c3cf509d69879 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Fri, 15 Jul 2011 14:47:09 +0100
Subject: [PATCH 7/8] perl: Add binding for guestfs_user_cancel.

---
 generator/generator_perl.ml |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/generator/generator_perl.ml b/generator/generator_perl.ml
index e2ec254..f42bc87 100644
--- a/generator/generator_perl.ml
+++ b/generator/generator_perl.ml
@@ -277,6 +277,12 @@ PREINIT:
  OUTPUT:
       RETVAL
 
+void
+user_cancel (g)
+      guestfs_h *g;
+ PPCODE:
+      guestfs_user_cancel (g);
+
 ";
 
   List.iter (
@@ -776,6 +782,11 @@ errnos:
    # mkdir failed because the directory exists already.
  }
 
+=item $h->user_cancel ();
+
+Cancel current transfer.  This is safe to call from Perl signal
+handlers and threads.
+
 =cut
 
 ";
-- 
1.7.5.2

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw
-------------- next part --------------
>From 1e9e351f178dcb425c4796c90fe7c8c5c250d100 Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones at redhat.com>
Date: Fri, 15 Jul 2011 14:57:00 +0100
Subject: [PATCH 8/8] ruby: Add binding for guestfs_user_cancel.

---
 generator/generator_ruby.ml |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/generator/generator_ruby.ml b/generator/generator_ruby.ml
index 7c8788d..a21334a 100644
--- a/generator/generator_ruby.ml
+++ b/generator/generator_ruby.ml
@@ -303,6 +303,26 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn)
   return r;
 }
 
+/*
+ * call-seq:
+ *   g.user_cancel() -> nil
+ *
+ * Call
+ *
+guestfs_user_cancel+[http://libguestfs.org/guestfs.3.html#guestfs_user_cancel]
+ * to cancel the current transfer.  This is safe to call from Ruby
+ * signal handlers and threads.
+ */
+static VALUE
+ruby_user_cancel (VALUE gv)
+{
+  guestfs_h *g;
+
+  Data_Get_Struct (gv, guestfs_h, g);
+  if (g)
+    guestfs_user_cancel (g);
+  return Qnil;
+}
+
 ";
 
   List.iter (
@@ -570,6 +590,8 @@ void Init__guestfs ()
                     ruby_set_event_callback, 2);
   rb_define_method (c_guestfs, \"delete_event_callback\",
                     ruby_delete_event_callback, 1);
+  rb_define_method (c_guestfs, \"user_cancel\",
+                    ruby_user_cancel, 0);
 
 ";
 
-- 
1.7.5.2