[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[Libguestfs] [PATCH 3/8] Add user cancellation to the C API.



-- 
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
>From 9ec9c97ce2ba46e56b304d3a367c86adc703160d Mon Sep 17 00:00:00 2001
From: "Richard W.M. Jones" <rjones 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]