From 9ec9c97ce2ba46e56b304d3a367c86adc703160d Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Fri, 15 Jul 2011 10:43:36 +0100 Subject: [PATCH] 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 + +#include +#include +#include +#include +#include +#include +#include + +#include + +#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 or C parameters in the generator). + +=head2 guestfs_user_cancel + + void guestfs_user_cancel (guestfs_h *g); + +C 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) is set to C, 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. + +In an interactive text-based program, you might call it from a +C signal handler so that pressing C<^C> cancels the current +operation. (You also need to call L 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.8.3.1