New API: guestfs_shutdown: Cleanly shutdown the backend.

The new API splits orderly close into a two-step process:

  if (guestfs_shutdown (g) == -1) {
    /* handle the error, eg. qemu error */
  }
  guestfs_close (g);

Note that the explicit shutdown step is only necessary in the case
where you have made changes to the disk image and want to handle write
errors.  Read the documentation for further information.

This change also:

 - deprecates guestfs_kill_subprocess

 - turns guestfs_kill_subprocess into the same as guestfs_shutdown

 - changes guestfish and other tools to call shutdown + close
   where necessary (not for read-only tools)

 - updates documentation

 - updates examples

(cherry picked from commit ffbf1475f7ae7c462db289ad4834391469e72edd)

3 files changed, 98 insertions, 60 deletions

diff --git a/src/guestfs.c b/src/guestfs.c
index d2c5b4eb..ef0ca167 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -172,8 +172,6 @@ guestfs_create (void)
 void
 guestfs_close (guestfs_h *g)
 {
-  int status, sig;
-
   if (g->state == NO_HANDLE) {
     /* Not safe to call ANY callbacks here, so ... */
     fprintf (stderr, _("guestfs_close: called twice on the same handle\n"));
@@ -202,10 +200,6 @@ guestfs_close (guestfs_h *g)
 
   debug (g, "closing guestfs handle %p (state %d)", g, g->state);
 
-  /* Try to sync if autosync flag is set. */
-  if (g->autosync && g->state == READY)
-    ignore_value (guestfs_internal_autosync (g));
-
   /* If we are valgrinding the daemon, then we *don't* want to kill
    * the subprocess because we want the final valgrind messages sent
    * when we close sockets below.  However for normal production use,
@@ -213,42 +207,10 @@ guestfs_close (guestfs_h *g)
    * daemon or qemu is not responding).
    */
 #ifndef VALGRIND_DAEMON
-  /* Kill the qemu subprocess. */
   if (g->state != CONFIG)
-    ignore_value (guestfs_kill_subprocess (g));
+    ignore_value (guestfs_shutdown (g));
 #endif
 
-  /* Close sockets. */
-  if (g->fd[0] >= 0)
-    close (g->fd[0]);
-  if (g->fd[1] >= 0)
-    close (g->fd[1]);
-  if (g->sock >= 0)
-    close (g->sock);
-  g->fd[0] = -1;
-  g->fd[1] = -1;
-  g->sock = -1;
-
-  /* Wait for subprocess(es) to exit. */
-  if (g->pid > 0) {
-    if (waitpid (g->pid, &status, 0) == -1)
-      perror ("waitpid (qemu)");
-    if (WIFEXITED (status) && WEXITSTATUS (status) != 0)
-      fprintf (stderr, "libguestfs: close: qemu failed (status %d)\n",
-               WEXITSTATUS (status));
-    else if (WIFSIGNALED (status)) {
-      sig = WTERMSIG (status);
-      fprintf (stderr, "libguestfs: close: qemu terminated by signal %d (%s)\n",
-               sig, strsignal (sig));
-    }
-    else if (WIFSTOPPED (status)) {
-      sig = WSTOPSIG (status);
-      fprintf (stderr, "libguestfs: close: qemu stopped by signal %d (%s)\n",
-               sig, strsignal (sig));
-    }
-  }
-  if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0);
-
   /* Run user close callbacks. */
   guestfs___call_callbacks_void (g, GUESTFS_EVENT_CLOSE);
 
@@ -285,6 +247,69 @@ guestfs_close (guestfs_h *g)
   free (g);
 }
 
+/* Shutdown the backend. */
+int
+guestfs__shutdown (guestfs_h *g)
+{
+  int ret = 0;
+  int status, sig;
+
+  if (g->state == CONFIG)
+    return 0;
+
+  /* Try to sync if autosync flag is set. */
+  if (g->autosync && g->state == READY) {
+    if (guestfs_internal_autosync (g) == -1)
+      ret = -1;
+  }
+
+  /* Signal qemu to shutdown cleanly, and kill the recovery process. */
+  if (g->pid > 0) {
+    debug (g, "sending SIGTERM to process %d", g->pid);
+    kill (g->pid, SIGTERM);
+  }
+  if (g->recoverypid > 0) kill (g->recoverypid, 9);
+
+  /* Close sockets. */
+  if (g->fd[0] >= 0)
+    close (g->fd[0]);
+  if (g->fd[1] >= 0)
+    close (g->fd[1]);
+  if (g->sock >= 0)
+    close (g->sock);
+  g->fd[0] = -1;
+  g->fd[1] = -1;
+  g->sock = -1;
+
+  /* Wait for subprocess(es) to exit. */
+  if (g->pid > 0) {
+    if (waitpid (g->pid, &status, 0) == -1) {
+      perrorf (g, "waitpid (qemu)");
+      ret = -1;
+    }
+    else if (WIFEXITED (status) && WEXITSTATUS (status) != 0) {
+      error (g, "qemu failed (status %d)", WEXITSTATUS (status));
+      ret = -1;
+    }
+    else if (WIFSIGNALED (status)) {
+      sig = WTERMSIG (status);
+      error (g, "qemu terminated by signal %d (%s)", sig, strsignal (sig));
+      ret = -1;
+    }
+    else if (WIFSTOPPED (status)) {
+      sig = WSTOPSIG (status);
+      error (g, "qemu stopped by signal %d (%s)", sig, strsignal (sig));
+      ret = -1;
+    }
+  }
+  if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0);
+
+  g->pid = g->recoverypid = 0;
+  g->state = CONFIG;
+
+  return ret;
+}
+
 /* Close all open handles (called from atexit(3)). */
 static void
 close_handles (void)
diff --git a/src/guestfs.pod b/src/guestfs.pod
index 95ff3409..b993abcd 100644
--- a/src/guestfs.pod
+++ b/src/guestfs.pod
@@ -14,6 +14,7 @@ guestfs - Library for accessing and modifying virtual machine images
  guestfs_mount (g, "/dev/sda1", "/");
  guestfs_touch (g, "/hello");
  guestfs_umount (g, "/");
+ guestfs_shutdown (g);
  guestfs_close (g);
 
  cc prog.c -o prog -lguestfs
@@ -81,13 +82,10 @@ this:
   */
  guestfs_touch (g, "/hello");
  
- /* This is only needed for libguestfs < 1.5.24.  Since then
-  * it is done automatically when you close the handle.  See
-  * discussion of autosync in this page.
-  */
- guestfs_sync (g);
+ /* Synchronize the disk.  This is the opposite of guestfs_launch. */
+ guestfs_shutdown (g);
  
- /* Close the handle 'g'. */
+ /* Close and free the handle 'g'. */
  guestfs_close (g);
 
 The code above doesn't include any error checking.  In real code you
@@ -1343,12 +1341,37 @@ L</ERROR HANDLING> section below.
  void guestfs_close (guestfs_h *g);
 
 This closes the connection handle and frees up all resources used.
+If a close callback was set on the handle, then it is called.
 
-If autosync was set on the handle and the handle was launched, then
-this implicitly calls various functions to unmount filesystems and
-sync the disk.  See L</guestfs_set_autosync> for more details.
+The correct way to close the handle is:
 
-If a close callback was set on the handle, then it is called.
+ if (guestfs_shutdown (g) == -1) {
+   /* handle write errors here */
+ }
+ guestfs_close (g);
+
+L</guestfs_shutdown> is only needed if B<all> of the following are true:
+
+=over 4
+
+=item 1
+
+one or more disks were added in read-write mode, I<and>
+
+=item 2
+
+guestfs_launch was called, I<and>
+
+=item 3
+
+you made some changes, I<and>
+
+=item 4
+
+you have a way to handle write errors (eg. by exiting with an
+error code or reporting something to the user).
+
+=back
 
 =head1 ERROR HANDLING
 
diff --git a/src/launch.c b/src/launch.c
index 466ec89c..a2c49458 100644
--- a/src/launch.c
+++ b/src/launch.c
@@ -1451,17 +1451,7 @@ guestfs__wait_ready (guestfs_h *g)
 int
 guestfs__kill_subprocess (guestfs_h *g)
 {
-  if (g->state == CONFIG) {
-    error (g, _("no subprocess to kill"));
-    return -1;
-  }
-
-  debug (g, "sending SIGTERM to process %d", g->pid);
-
-  if (g->pid > 0) kill (g->pid, SIGTERM);
-  if (g->recoverypid > 0) kill (g->recoverypid, 9);
-
-  return 0;
+  return guestfs__shutdown (g);
 }
 
 /* Access current state. */