Libguestfs - Aug 2010 - [REPOST] guestfsd core capture, and virt-rescue clean shutdown

This is a repost of a previously posted set. It's been updated following
review
comments.

[PATCH 1/4] Add a core_pattern debug command
Added missing return statements.

[PATCH 2/4] Call sync after guestfsd exits
Remove redundant sleep 1; sync

[PATCH 3/4] Shut down the appliance cleanly
[PATCH 4/4] Ignore launch() error in virt-rescue
These were previously 2 patches. They are otherwise unchanged.

This adds a new debug command, core_pattern, which writes a new pattern for
coredump files to the appliance kernel, and sets the daemon's hard and soft
core
limits to infinity.
---
 daemon/debug.c |   46 ++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/daemon/debug.c b/daemon/debug.c
index 0867ccd..37d5237 100644
--- a/daemon/debug.c
+++ b/daemon/debug.c
@@ -26,6 +26,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <dirent.h>
+#include <sys/resource.h>
 
 #include "../src/guestfs_protocol.h"
 #include "daemon.h"
@@ -52,6 +53,7 @@ static char *debug_ls (const char *subcmd, int argc, char
*const *const argv);
 static char *debug_ll (const char *subcmd, int argc, char *const *const argv);
 static char *debug_segv (const char *subcmd, int argc, char *const *const
argv);
 static char *debug_sh (const char *subcmd, int argc, char *const *const argv);
+static char *debug_core_pattern (const char *subcmd, int argc, char *const
*const argv);
 
 static struct cmd cmds[] = {
   { "help", debug_help },
@@ -61,6 +63,7 @@ static struct cmd cmds[] = {
   { "ll", debug_ll },
   { "segv", debug_segv },
   { "sh", debug_sh },
+  { "core_pattern", debug_core_pattern },
   { NULL, NULL }
 };
 #endif
@@ -338,6 +341,49 @@ debug_ll (const char *subcmd, int argc, char *const *const
argv)
   return out;
 }
 
+/* Enable core dumping to the given core pattern.
+ * Note that this pattern is relative to any chroot of the process which
+ * crashes. This means that if you want to write the core file to the
guest's
+ * storage the pattern must start with /sysroot only if the command which
+ * crashes doesn't chroot.
+ */
+static char *
+debug_core_pattern (const char *subcmd, int argc, char *const *const argv)
+{
+  if (argc < 1) {
+    reply_with_error ("core_pattern: expecting a core pattern");
+  }
+
+  const char *pattern = argv[0];
+  const size_t pattern_len = strlen(pattern);
+
+#define CORE_PATTERN "/proc/sys/kernel/core_pattern"
+  int fd = open (CORE_PATTERN, O_WRONLY);
+  if (fd == -1) {
+    reply_with_error ("open: " CORE_PATTERN);
+    return NULL;
+  }
+  if (write (fd, pattern, pattern_len) < (ssize_t) pattern_len) {
+    reply_with_error ("write: " CORE_PATTERN);
+    return NULL;
+  }
+  if (close (fd) == -1) {
+    reply_with_error ("close: " CORE_PATTERN);
+    return NULL;
+  }
+
+  struct rlimit limit = {
+    .rlim_cur = RLIM_INFINITY,
+    .rlim_max = RLIM_INFINITY
+  };
+  if (setrlimit (RLIMIT_CORE, &limit) == -1) {
+    reply_with_error ("setrlimit (RLIMIT_CORE)");
+    return NULL;
+  }
+
+  return strdup("ok");
+}
+
 #endif /* ENABLE_DEBUG_COMMAND */
 
 #if ENABLE_DEBUG_COMMAND
-- 
1.7.2.2

Core files are not reliably written to disk if guestfsd dumps core. This patch
makes libguestfs do the same appliance cleanup for guestfsd and virt-rescue,
which seems to fix the matter.

It also removes a redundant sleep and additional sync when exiting virt-rescue.
---
 appliance/init |   45 ++++++++++++++++++++++++---------------------
 1 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/appliance/init b/appliance/init
index 6aeea0c..90da1cb 100755
--- a/appliance/init
+++ b/appliance/init
@@ -88,27 +88,30 @@ if grep -sq guestfs_verbose=1 /proc/cmdline; then
 fi
 
 if ! grep -sq guestfs_rescue=1 /proc/cmdline; then
-  exec guestfsd -f
+  # The host will kill qemu abruptly if guestfsd shuts down normally
+  guestfsd -f
+
+  # Otherwise we try to clean up gracefully. For example, this ensures that a
+  # core dump generated by the guest daemon will be written to disk.
+else
+  # Use appliance in rescue mode, also used by the virt-rescue command.
+  eval $(grep -Eo 'TERM=[^[:space:]]+' /proc/cmdline)
+  PS1='><rescue> '
+  export TERM PS1
+  echo
+  echo "------------------------------------------------------------"
+  echo
+  echo "Welcome to virt-rescue, the libguestfs rescue shell."
+  echo
+  echo "Note: The contents of / are the rescue appliance."
+  echo "You have to mount the guest's partitions under /sysroot"
+  echo "before you can examine them."
+  echo
+  bash -i
+  echo
+  echo "virt-rescue: Syncing the disk now before exiting ..."
+  echo "(Don't worry if you see a 'Kernel panic' message
below)"
+  echo
 fi
 
-# Use appliance in rescue mode, also used by the virt-rescue command.
-eval $(grep -Eo 'TERM=[^[:space:]]+' /proc/cmdline)
-PS1='><rescue> '
-export TERM PS1
-echo
-echo "------------------------------------------------------------"
-echo
-echo "Welcome to virt-rescue, the libguestfs rescue shell."
-echo
-echo "Note: The contents of / are the rescue appliance."
-echo "You have to mount the guest's partitions under /sysroot"
-echo "before you can examine them."
-echo
-bash -i
-echo
-echo "virt-rescue: Syncing the disk now before exiting ..."
-echo "(Don't worry if you see a 'Kernel panic' message
below)"
-echo
-sync
-sleep 1
 sync
-- 
1.7.2.2

When guestfsd exits, or the user exits the virt-rescue shell, the init script
exits which causes the kernel to panic. This isn't really a functional
issue, as
all useful work is done by this point. However, it does cause virt-rescue to
display an unsightly error message.

This patch adds a new utility, guestfs_poweroff, to the appliance. This powers
off (reboots really: read the comment) the appliance, preventing init from
exiting, and therefore the kernel panic.
---
 .gitignore                                   |    1 +
 appliance/Makefile.am                        |    6 ++++--
 appliance/debian/modules/y0_install-guestfsd |    2 ++
 appliance/guestfs_poweroff.c                 |   14 ++++++++++++++
 appliance/init                               |    2 +-
 appliance/update.sh.in                       |    4 ++++
 po/POTFILES.in                               |    1 +
 7 files changed, 27 insertions(+), 3 deletions(-)
 create mode 100644 appliance/guestfs_poweroff.c

diff --git a/.gitignore b/.gitignore
index 094acb3..abd88bc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -8,6 +8,7 @@ appliance/debian/debirf-libguestfs*.cgz
 appliance/debian/root/
 appliance/debian/vmlinuz-*
 appliance/debian/debirf.conf
+appliance/guestfs_poweroff
 appliance/initramfs.*.img
 appliance/kmod.whitelist
 appliance/make.sh
diff --git a/appliance/Makefile.am b/appliance/Makefile.am
index bbf3c24..ccf7907 100644
--- a/appliance/Makefile.am
+++ b/appliance/Makefile.am
@@ -46,6 +46,8 @@ superminfs_DATA = \
 	supermin.d/hostfiles
 endif
 
+noinst_PROGRAMS = guestfs_poweroff
+
 # Don't change these names - they must be the same as in '*.sh'
scripts.
 INITRAMFSIMG = initramfs.$(REPO).$(host_cpu).img
 VMLINUZ = vmlinuz.$(REPO).$(host_cpu)
@@ -67,7 +69,7 @@ make.sh: make.sh.in
 	chmod +x $@-t
 	mv $@-t $@
 
-$(INITRAMFSIMG): $(top_builddir)/initramfs/fakeroot.log
$(top_builddir)/daemon/guestfsd init update.sh
+$(INITRAMFSIMG): $(top_builddir)/initramfs/fakeroot.log
$(top_builddir)/daemon/guestfsd init guestfs_poweroff update.sh
 	rm -f $@
 	bash update.sh
 	touch $@
@@ -99,7 +101,7 @@ supermin.d/daemon.img: $(INITRAMFSIMG)
 	mkdir -p supermin.d
 	rm -f $@ $@-t
 	(cd $(top_builddir)/initramfs && \
-	  echo -e "sbin\nsbin/guestfsd" | cpio --quiet -o -H newc ) >
$@-t
+	  echo -e "sbin\nsbin/guestfsd\nsbin/guestfs_poweroff" | cpio
--quiet -o -H newc ) > $@-t
 	mv $@-t $@
 endif
 
diff --git a/appliance/debian/modules/y0_install-guestfsd
b/appliance/debian/modules/y0_install-guestfsd
index 2d895a0..c2b812a 100755
--- a/appliance/debian/modules/y0_install-guestfsd
+++ b/appliance/debian/modules/y0_install-guestfsd
@@ -37,4 +37,6 @@ rm -rf "$DEBIRF_ROOT"/usr/share/man/
 # Install the actual appliance:
 echo $PWD
 install -o root -g root -m 0755 ../daemon/guestfsd
"$DEBIRF_ROOT"/sbin/guestfsd
+install -o root -g root -m 0755 ../appliance/guestfs_poweroff \
+                                "$DEBIRF_ROOT"/sbin/guestfs_poweroff
 install -o root -g root -m 0755 init "$DEBIRF_ROOT"/sbin/init
diff --git a/appliance/guestfs_poweroff.c b/appliance/guestfs_poweroff.c
new file mode 100644
index 0000000..7e18839
--- /dev/null
+++ b/appliance/guestfs_poweroff.c
@@ -0,0 +1,14 @@
+#include <stdio.h>
+#include <sys/reboot.h>
+
+int main(int argc, char **argv)
+{
+    /* We actually hard reboot here rather than power off. For some reason
+     * reboot causes the qemu process to die, whereas poweroff doesn't.
+     *
+     * This should not return */
+    reboot(RB_AUTOBOOT);
+
+    perror("Power off failed");
+    return 1;
+}
diff --git a/appliance/init b/appliance/init
index 90da1cb..4ed93b8 100755
--- a/appliance/init
+++ b/appliance/init
@@ -110,8 +110,8 @@ else
   bash -i
   echo
   echo "virt-rescue: Syncing the disk now before exiting ..."
-  echo "(Don't worry if you see a 'Kernel panic' message
below)"
   echo
 fi
 
 sync
+exec /sbin/guestfs_poweroff
diff --git a/appliance/update.sh.in b/appliance/update.sh.in
index 0222a75..9cb3450 100755
--- a/appliance/update.sh.in
+++ b/appliance/update.sh.in
@@ -33,6 +33,10 @@ if [ "@DIST@" = "REDHAT" ]; then
   # Copy the daemon into the filesystem.
   @FEBOOTSTRAP_INSTALL@ initramfs daemon/guestfsd /sbin/guestfsd 0755 root.root
 
+  # Copy guestfs_poweroff into the filesystem
+  febootstrap-install initramfs appliance/guestfs_poweroff \
+                                /sbin/guestfs_poweroff 0755 root.root
+
   # Generate final image.
   @FEBOOTSTRAP_TO_INITRAMFS@ initramfs > $output-t
   mv $output-t $output
diff --git a/po/POTFILES.in b/po/POTFILES.in
index e5fb857..b2f7f26 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -1,3 +1,4 @@
+appliance/guestfs_poweroff.c
 daemon/augeas.c
 daemon/available.c
 daemon/base64.c
-- 
1.7.2.2

launch() expects guestfsd to start, which it never does in virt-rescue, so it
always returns an error about the appliance shutting down unexpectedly.
---
 tools/virt-rescue |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/tools/virt-rescue b/tools/virt-rescue
index 1f292f6..f466b43 100755
--- a/tools/virt-rescue
+++ b/tools/virt-rescue
@@ -214,7 +214,8 @@ $g->set_append ($str);
 
 # Run the appliance.  This won't return until the user quite the
 # appliance.
-$g->launch ();
+# This will definitely return an error because we don't run the daemon
+eval { $g->launch (); };
 
 exit 0;
 
-- 
1.7.2.2