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 ec8e3b6cad
  Author: Richard W.M. Jones <rjones@redhat.com>
  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.
This commit is contained in:
Richard W.M. Jones
2012-09-04 14:37:44 +01:00
parent 2e36adf83c
commit 8248a346d7

View File

@@ -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;