| Message ID | 1336679442-19198-3-git-send-email-lcapitulino@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 05/10/2012 01:50 PM, Luiz Capitulino wrote: > This fixes a bug where qemu-ga doesn't suspend the guest because it > fails to detect suspend support even when the guest does support > suspend. This happens because of the way qemu-ga fds are managed in > daemon mode. > > When starting qemu-ga with --daemon, become_daemon() will close all > standard fds. This will cause qemu-ga to end up with the following > fds (if started with 'qemu-ga --daemon'): > > 0 -> /dev/vport0p1 > 3 -> /run/qemu-ga.pid > > Then a guest-suspend-* function is issued. They call bios_supports_mode(), > which will call pipe(), and qemu-ga's fd will be: > > 0 -> /dev/vport0p1 > 1 -> pipe:[16247] > 2 -> pipe:[16247] > 3 -> /run/qemu-ga.pid Very nasty. > To solve this problem we have to reopen standard fds to /dev/null > in become_daemon(), instead of closing them. Yes, POSIX warns that applications should never call exec() with fd 0, 1, or 2 closed, at least not if the application wants the child to behave in a conforming environment. > > Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > qemu-ga.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/qemu-ga.c b/qemu-ga.c > index 7896565..c64bc71 100644 > --- a/qemu-ga.c > +++ b/qemu-ga.c > @@ -445,9 +445,9 @@ static void become_daemon(const char *pidfile) > goto fail; > } > > - close(STDIN_FILENO); > - close(STDOUT_FILENO); > - close(STDERR_FILENO); > + reopen_fd_to_null(STDIN_FILENO); > + reopen_fd_to_null(STDOUT_FILENO); > + reopen_fd_to_null(STDERR_FILENO); Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/qemu-ga.c b/qemu-ga.c index 7896565..c64bc71 100644 --- a/qemu-ga.c +++ b/qemu-ga.c @@ -445,9 +445,9 @@ static void become_daemon(const char *pidfile) goto fail; } - close(STDIN_FILENO); - close(STDOUT_FILENO); - close(STDERR_FILENO); + reopen_fd_to_null(STDIN_FILENO); + reopen_fd_to_null(STDOUT_FILENO); + reopen_fd_to_null(STDERR_FILENO); return; fail:
This fixes a bug where qemu-ga doesn't suspend the guest because it fails to detect suspend support even when the guest does support suspend. This happens because of the way qemu-ga fds are managed in daemon mode. When starting qemu-ga with --daemon, become_daemon() will close all standard fds. This will cause qemu-ga to end up with the following fds (if started with 'qemu-ga --daemon'): 0 -> /dev/vport0p1 3 -> /run/qemu-ga.pid Then a guest-suspend-* function is issued. They call bios_supports_mode(), which will call pipe(), and qemu-ga's fd will be: 0 -> /dev/vport0p1 1 -> pipe:[16247] 2 -> pipe:[16247] 3 -> /run/qemu-ga.pid bios_supports_mode() forks off a child and blocks waiting for the child to write something to the pipe. The child, however, closes its reading end of the pipe _and_ reopen all standard fds to /dev/null. This will cause the child's fds to be: 0 -> /dev/null 1 -> /dev/null 2 -> /dev/null 3 -> /run/qemu-ga.pid In other words, the child's writing end of the pipe is now /dev/null. It writes there and exits. The parent process (blocked on read()) will get an EOF and interpret this as "something unexpected happened in the child, let's assume the guest doesn't support suspend". And suspend will fail. To solve this problem we have to reopen standard fds to /dev/null in become_daemon(), instead of closing them. Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com> --- qemu-ga.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)