[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
On Thu, Aug 26, 2010 at 12:11:58PM +0100, Matthew Booth wrote:
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 },
These are in alphabetical order (apart from "help") so we should keep them in that order.
+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"); + }
Missing 'return NULL;'.
+ 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);
These should (all 4 of them) use reply_with_perror, since I assume you want to communicate the reason for the failure,
+ return strdup("ok");
Interesting .. hadn't thought about what we might return for commands which succeed without returning anything, but I guess "ok" is as good as anything. It's only a debug command after all. However I'd prefer: char *ret = strdup ("ok"); if (ret == NULL) { reply_with_perror ("strdup"); return NULL; } return ret; In general this patch is (still) good and we should integrate this functionality as soon as possible. Post an updated patch which fixes all the above and we should be able to get it in straightaway. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
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 | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 53 insertions(+), 0 deletions(-) diff --git a/daemon/debug.c b/daemon/debug.c index 0867ccd..c0d87da 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,9 +53,11 @@ 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 }, + { "core_pattern", debug_core_pattern }, { "env", debug_env }, { "fds", debug_fds }, { "ls", debug_ls }, @@ -338,6 +341,56 @@ 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"); + return NULL; + } + + 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_perror ("open: " CORE_PATTERN); + return NULL; + } + if (write (fd, pattern, pattern_len) < (ssize_t) pattern_len) { + reply_with_perror ("write: " CORE_PATTERN); + return NULL; + } + if (close (fd) == -1) { + reply_with_perror ("close: " CORE_PATTERN); + return NULL; + } + + struct rlimit limit = { + .rlim_cur = RLIM_INFINITY, + .rlim_max = RLIM_INFINITY + }; + if (setrlimit (RLIMIT_CORE, &limit) == -1) { + reply_with_perror ("setrlimit (RLIMIT_CORE)"); + return NULL; + } + + char *ret = strdup ("ok"); + if (NULL == ret) { + reply_with_perror ("strdup"); + return NULL; + } + + return ret; +} + #endif /* ENABLE_DEBUG_COMMAND */ #if ENABLE_DEBUG_COMMAND -- 1.7.2.2
On Thu, Aug 26, 2010 at 01:36:10PM +0100, Matthew Booth wrote:
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.
Thanks, THIS is the patch that I have pushed. Rich. -- 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/
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
On Thu, Aug 26, 2010 at 12:11:59PM +0100, Matthew Booth wrote:
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
ACK. I'll push this once 1/4 is reposted. Rich. -- 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
On Thu, Aug 26, 2010 at 12:11:59PM +0100, Matthew Booth wrote:
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.
Thanks, I've added that. Rich. -- 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
On Thu, Aug 26, 2010 at 02:01:38PM +0100, Richard W.M. Jones wrote:
On Thu, Aug 26, 2010 at 12:11:59PM +0100, Matthew Booth wrote:
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.
Thanks, I've added that.
No, I didn't ... replied to the wrong email. 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://et.redhat.com/~rjones/libguestfs/ See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
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
On Thu, Aug 26, 2010 at 12:12:00PM +0100, Matthew Booth wrote:
+ /* 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);
We run qemu with -no-reboot which turns reboots into poweroffs. As to why poweroff doesn't work, I'd guess it's something to do with the fact that we disable ACPI support in the kernel.
+ perror("Power off failed"); + return 1;
This should probably be exit (or return) EXIT_FAILURE. This patch now looks good in general. I'm going to play with it on my machine to make sure it works and there are no regressions that I can find. Rich. -- 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
danpb pointed out on IRC that /sbin/reboot with -f will reboot directly without using init. This works for us, resulting in a much smaller, 2 line patch :) Matt
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 causes the appliance to power off cleanly before the init script exits. Note it actually does a reboot rather than a poweroff. This is because ACPI is disabled in the appliance, meaning poweroff doesn't work, but qemu is configured not to restart on reboot. --- appliance/init | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/appliance/init b/appliance/init index 90da1cb..1c01195 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 +/sbin/reboot -f -- 1.7.2.2
On Thu, Aug 26, 2010 at 02:34:44PM +0100, Matthew Booth wrote:
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 causes the appliance to power off cleanly before the init script exits. Note it actually does a reboot rather than a poweroff. This is because ACPI is disabled in the appliance, meaning poweroff doesn't work, but qemu is configured not to restart on reboot. --- appliance/init | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/appliance/init b/appliance/init index 90da1cb..1c01195 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 +/sbin/reboot -f -- 1.7.2.2
Thanks. Patches 2/4 and 3/4 (this one) will be pushed shortly. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
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
On Thu, Aug 26, 2010 at 12:12:01PM +0100, Matthew Booth wrote:
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 (); };
It seems a shame to lose the error message here, for example in the cause where launch really doesn't work, eg. it can't find the appliance. How about: eval { $g->launch (); }; print $@ if $@; Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
On 26/08/10 13:20, Richard W.M. Jones wrote:
On Thu, Aug 26, 2010 at 12:12:01PM +0100, Matthew Booth wrote:
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 (); };
It seems a shame to lose the error message here, for example in the cause where launch really doesn't work, eg. it can't find the appliance. How about:
eval { $g->launch (); }; print $@ if $@;
Well that will always display an error message about a child process dying unexpectedly. I'll post an updated patch which ignores ECHILD. I've confirmed that this display an error when the appliance can't be found. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
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 | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-) diff --git a/tools/virt-rescue b/tools/virt-rescue index 1f292f6..c84fb7f 100755 --- a/tools/virt-rescue +++ b/tools/virt-rescue @@ -19,6 +19,7 @@ use warnings; use strict; +use Errno; use Sys::Guestfs; use Sys::Guestfs::Lib qw(open_guest); use Pod::Usage; @@ -214,7 +215,11 @@ $g->set_append ($str); # Run the appliance. This won't return until the user quite the # appliance. -$g->launch (); +eval { $g->launch (); }; + +# launch() expects guestfsd to start. However, virt-rescue doesn't run guestfsd, +# so this will always fail with ECHILD when the appliance exits unexpectedly. +die ($@) unless ($!{ECHILD}); exit 0; -- 1.7.2.2
participants (2)
-
Matthew Booth -
Richard W.M. Jones