From 8248a346d71ed7fc964595f6e973ff6593cdc217 Mon Sep 17 00:00:00 2001 From: "Richard W.M. Jones" Date: Tue, 4 Sep 2012 14:37:44 +0100 Subject: proto: Don't set g->fd[] to /dev/null in direct mode, fixing virt-rescue (RHBZ#853159). https://bugzilla.redhat.com/show_bug.cgi?id=853159 git bisect pointed to the following commit: commit ec8e3b6cad170d08ac18b580792dfb137eb171dc Author: Richard W.M. Jones Date: Fri Jul 20 14:24:10 2012 +0100 launch: Abstract attach method operations. g->attach_ops points to a structure which contains the operations supported by each attach method backend (ie. appliance, unix, etc.). Since that commit was essentially just code motion, it wasn't clear why virt-rescue should be affected by it. In fact the reason is as follows: (1) In direct mode, we don't need g->fd[] (which would normally be connected to the stdin/stdout of qemu). So we opened them on /dev/null so they had some value. (2) accept_from_daemon / read_log_message_or_eof reads from g->fd[1]. Since this is connected to /dev/null, it always reads EOF. (3) This would cause child_cleanup to be called. This is completely unintentional: we don't want to cleanup the child at this point, even in direct mode. (4) Prior to the commit above, child_cleanup first waited for the process to exit (ie. waitpid). This happened to work, since we are effectively waiting for the user to exit virt-rescue. (5) After the commit above, the order of operations was changed so that we first killed qemu before waiting for it. This broke virt-rescue. The fix is to change direct mode so that it leaves g->fd[]'s as -1. The rest of the protocol code can deal with this situation -- it ignores the log fd instead of trying to read from it. --- src/launch-appliance.c | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/launch-appliance.c b/src/launch-appliance.c index 328cd196..134ddde0 100644 --- a/src/launch-appliance.c +++ b/src/launch-appliance.c @@ -602,19 +602,6 @@ launch_appliance (guestfs_h *g, const char *arg) g->fd[0] = wfd[1]; /* stdin of child */ g->fd[1] = rfd[0]; /* stdout of child */ wfd[1] = rfd[0] = -1; - } else { - g->fd[0] = open ("/dev/null", O_RDWR|O_CLOEXEC); - if (g->fd[0] == -1) { - perrorf (g, "open /dev/null"); - goto cleanup1; - } - g->fd[1] = dup (g->fd[0]); - if (g->fd[1] == -1) { - perrorf (g, "dup"); - close (g->fd[0]); - g->fd[0] = -1; - goto cleanup1; - } } g->state = LAUNCHING; -- cgit