Richard W.M. Jones
2013-Mar-07 11:44 UTC
[Libguestfs] [PATCH 0/4] Small refactorings of the protocol layer.
As the start of work to add remote support, I'm taking a close look at the protocol layer in the library. These are some small cleanups. Rich.
Richard W.M. Jones
2013-Mar-07 11:44 UTC
[Libguestfs] [PATCH 1/4] launch: appliance: Use socketpair instead of pipe for qemu stdin/stdout.
From: "Richard W.M. Jones" <rjones at redhat.com>
The libvirt backend already uses a Unix socket for the appliance
console, and so for the libvirt backend the fields g->fd[0] == g->fd[1].
Change the appliance backend to use a socketpair, so we need just a
single file descriptor for qemu stdin/stdout (ie. appliance console).
Consequently we can remove the array int fd[2] in the handle and
replace it with a single file descriptor.
---
src/guestfs-internal.h | 2 +-
src/handle.c | 12 ++++--------
src/launch-appliance.c | 49 +++++++++++++++++++------------------------------
src/launch-libvirt.c | 18 ++++--------------
src/launch-unix.c | 7 +++----
src/proto.c | 36 +++++++++++++++++-------------------
6 files changed, 48 insertions(+), 76 deletions(-)
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 793549f..4c632e8 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -263,7 +263,7 @@ struct guestfs_h
int unique;
/*** Protocol. ***/
- int fd[2]; /* Stdin/stdout of qemu. */
+ int fd; /* Stdin/stdout of qemu. */
int sock; /* Daemon communications socket. */
int msg_next_serial;
diff --git a/src/handle.c b/src/handle.c
index 0e66d1a..cefe385 100644
--- a/src/handle.c
+++ b/src/handle.c
@@ -91,8 +91,7 @@ guestfs_create_flags (unsigned flags, ...)
g->state = CONFIG;
- g->fd[0] = -1;
- g->fd[1] = -1;
+ g->fd = -1;
g->sock = -1;
guestfs___init_error_handler (g);
@@ -367,14 +366,11 @@ shutdown_backend (guestfs_h *g, int check_for_errors)
}
/* Close sockets. */
- if (g->fd[0] >= 0)
- close (g->fd[0]);
- if (g->fd[1] >= 0)
- close (g->fd[1]);
+ if (g->fd >= 0)
+ close (g->fd);
if (g->sock >= 0)
close (g->sock);
- g->fd[0] = -1;
- g->fd[1] = -1;
+ g->fd = -1;
g->sock = -1;
if (g->attach_ops->shutdown (g, check_for_errors) == -1)
diff --git a/src/launch-appliance.c b/src/launch-appliance.c
index a86a8cc..4016d61 100644
--- a/src/launch-appliance.c
+++ b/src/launch-appliance.c
@@ -167,7 +167,7 @@ static int
launch_appliance (guestfs_h *g, const char *arg)
{
int r;
- int wfd[2], rfd[2];
+ int sv[2];
char guestfsd_sock[256];
struct sockaddr_un addr;
CLEANUP_FREE char *kernel = NULL, *initrd = NULL, *appliance = NULL;
@@ -235,8 +235,8 @@ launch_appliance (guestfs_h *g, const char *arg)
}
if (!g->direct) {
- if (pipe (wfd) == -1 || pipe (rfd) == -1) {
- perrorf (g, "pipe");
+ if (socketpair (AF_LOCAL, SOCK_STREAM, 0, sv) == -1) {
+ perrorf (g, "socketpair");
goto cleanup0;
}
}
@@ -248,10 +248,8 @@ launch_appliance (guestfs_h *g, const char *arg)
if (r == -1) {
perrorf (g, "fork");
if (!g->direct) {
- close (wfd[0]);
- close (wfd[1]);
- close (rfd[0]);
- close (rfd[1]);
+ close (sv[0]);
+ close (sv[1]);
}
goto cleanup0;
}
@@ -484,17 +482,16 @@ launch_appliance (guestfs_h *g, const char *arg)
/* Set up stdin, stdout, stderr. */
close (0);
close (1);
- close (wfd[1]);
- close (rfd[0]);
+ close (sv[0]);
/* Stdin. */
- if (dup (wfd[0]) == -1) {
+ if (dup (sv[1]) == -1) {
dup_failed:
perror ("dup failed");
_exit (EXIT_FAILURE);
}
/* Stdout. */
- if (dup (rfd[1]) == -1)
+ if (dup (sv[1]) == -1)
goto dup_failed;
/* Particularly since qemu 0.15, qemu spews all sorts of debug
@@ -502,11 +499,10 @@ launch_appliance (guestfs_h *g, const char *arg)
* not confuse casual users, so send stderr to the pipe as well.
*/
close (2);
- if (dup (rfd[1]) == -1)
+ if (dup (sv[1]) == -1)
goto dup_failed;
- close (wfd[0]);
- close (rfd[1]);
+ close (sv[1]);
}
/* Dump the command line (after setting up stderr above). */
@@ -600,19 +596,16 @@ launch_appliance (guestfs_h *g, const char *arg)
}
if (!g->direct) {
- /* Close the other ends of the pipe. */
- close (wfd[0]);
- close (rfd[1]);
+ /* Close the other end of the socketpair. */
+ close (sv[1]);
- if (fcntl (wfd[1], F_SETFL, O_NONBLOCK) == -1 ||
- fcntl (rfd[0], F_SETFL, O_NONBLOCK) == -1) {
+ if (fcntl (sv[0], F_SETFL, O_NONBLOCK) == -1) {
perrorf (g, "fcntl");
goto cleanup1;
}
- g->fd[0] = wfd[1]; /* stdin of child */
- g->fd[1] = rfd[0]; /* stdout of child */
- wfd[1] = rfd[0] = -1;
+ g->fd = sv[0]; /* stdin of child */
+ sv[0] = -1;
}
g->state = LAUNCHING;
@@ -680,18 +673,14 @@ launch_appliance (guestfs_h *g, const char *arg)
return 0;
cleanup1:
- if (!g->direct) {
- if (wfd[1] >= 0) close (wfd[1]);
- if (rfd[1] >= 0) close (rfd[0]);
- }
+ if (!g->direct && sv[0] >= 0)
+ close (sv[0]);
if (g->app.pid > 0) kill (g->app.pid, 9);
if (g->app.recoverypid > 0) kill (g->app.recoverypid, 9);
if (g->app.pid > 0) waitpid (g->app.pid, NULL, 0);
if (g->app.recoverypid > 0) waitpid (g->app.recoverypid, NULL, 0);
- if (g->fd[0] >= 0) close (g->fd[0]);
- if (g->fd[1] >= 0) close (g->fd[1]);
- g->fd[0] = -1;
- g->fd[1] = -1;
+ if (g->fd >= 0) close (g->fd);
+ g->fd = -1;
g->app.pid = 0;
g->app.recoverypid = 0;
memset (&g->launch_t, 0, sizeof g->launch_t);
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index 5a69ced..caad5d4 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -399,13 +399,7 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
goto cleanup;
}
console = -1;
- g->fd[0] = r; /* This is the accepted console socket. */
-
- g->fd[1] = dup (g->fd[0]);
- if (g->fd[1] == -1) {
- perrorf (g, "dup");
- goto cleanup;
- }
+ g->fd = r; /* This is the accepted console socket. */
/* Wait for libvirt domain to start and to connect back to us via
* virtio-serial and send the GUESTFS_LAUNCH_FLAG message.
@@ -481,13 +475,9 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
if (console >= 0)
close (console);
- if (g->fd[0] >= 0) {
- close (g->fd[0]);
- g->fd[0] = -1;
- }
- if (g->fd[1] >= 0) {
- close (g->fd[1]);
- g->fd[1] = -1;
+ if (g->fd >= 0) {
+ close (g->fd);
+ g->fd = -1;
}
if (g->sock >= 0) {
close (g->sock);
diff --git a/src/launch-unix.c b/src/launch-unix.c
index 9bd64fa..a7a3ae2 100644
--- a/src/launch-unix.c
+++ b/src/launch-unix.c
@@ -46,11 +46,10 @@ launch_unix (guestfs_h *g, const char *sockpath)
return -1;
}
- /* Set these to nothing so we don't try to read from random file
- * descriptors.
+ /* Set this to nothing so we don't try to read from a random file
+ * descriptor.
*/
- g->fd[0] = -1;
- g->fd[1] = -1;
+ g->fd = -1;
if (g->verbose)
guestfs___print_timestamped_message (g, "connecting to %s",
sockpath);
diff --git a/src/proto.c b/src/proto.c
index 3d00b2b..57cb609 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -161,11 +161,9 @@ child_cleanup (guestfs_h *g)
debug (g, "child_cleanup: %p: child process died", g);
g->attach_ops->shutdown (g, 0);
- if (g->fd[0] >= 0) close (g->fd[0]);
- if (g->fd[1] >= 0) close (g->fd[1]);
+ if (g->fd >= 0) close (g->fd);
close (g->sock);
- g->fd[0] = -1;
- g->fd[1] = -1;
+ g->fd = -1;
g->sock = -1;
memset (&g->launch_t, 0, sizeof g->launch_t);
guestfs___free_drives (g);
@@ -385,12 +383,12 @@ send_to_daemon (guestfs_h *g, const void *v_buf, size_t n)
FD_ZERO (&rset);
FD_ZERO (&wset);
- if (g->fd[1] >= 0) /* Read qemu stdout for log messages
& EOF. */
- FD_SET (g->fd[1], &rset);
+ if (g->fd >= 0) /* Read qemu stdout for log messages
& EOF. */
+ FD_SET (g->fd, &rset);
FD_SET (g->sock, &rset); /* Read socket for cancellation &
EOF. */
FD_SET (g->sock, &wset); /* Write to socket to send the data. */
- int max_fd = MAX (g->sock, g->fd[1]);
+ int max_fd = MAX (g->sock, g->fd);
while (n > 0) {
rset2 = rset;
@@ -403,8 +401,8 @@ send_to_daemon (guestfs_h *g, const void *v_buf, size_t n)
return -1;
}
- if (g->fd[1] >= 0 && FD_ISSET (g->fd[1], &rset2)) {
- if (read_log_message_or_eof (g, g->fd[1], 0) == -1)
+ if (g->fd >= 0 && FD_ISSET (g->fd, &rset2)) {
+ if (read_log_message_or_eof (g, g->fd, 0) == -1)
return -1;
}
if (FD_ISSET (g->sock, &rset2)) {
@@ -516,11 +514,11 @@ recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void
**buf_rtn)
FD_ZERO (&rset);
- if (g->fd[1] >= 0) /* Read qemu stdout for log messages
& EOF. */
- FD_SET (g->fd[1], &rset);
+ if (g->fd >= 0) /* Read qemu stdout for log messages
& EOF. */
+ FD_SET (g->fd, &rset);
FD_SET (g->sock, &rset); /* Read socket for data & EOF. */
- max_fd = MAX (g->sock, g->fd[1]);
+ max_fd = MAX (g->sock, g->fd);
*size_rtn = 0;
*buf_rtn = NULL;
@@ -548,8 +546,8 @@ recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void
**buf_rtn)
return -1;
}
- if (g->fd[1] >= 0 && FD_ISSET (g->fd[1], &rset2)) {
- if (read_log_message_or_eof (g, g->fd[1], 0) == -1) {
+ if (g->fd >= 0 && FD_ISSET (g->fd, &rset2)) {
+ if (read_log_message_or_eof (g, g->fd, 0) == -1) {
free (*buf_rtn);
*buf_rtn = NULL;
return -1;
@@ -728,11 +726,11 @@ guestfs___accept_from_daemon (guestfs_h *g)
FD_ZERO (&rset);
- if (g->fd[1] >= 0) /* Read qemu stdout for log messages
& EOF. */
- FD_SET (g->fd[1], &rset);
+ if (g->fd >= 0) /* Read qemu stdout for log messages
& EOF. */
+ FD_SET (g->fd, &rset);
FD_SET (g->sock, &rset); /* Read socket for accept. */
- int max_fd = MAX (g->sock, g->fd[1]);
+ int max_fd = MAX (g->sock, g->fd);
int sock = -1;
while (sock == -1) {
@@ -760,8 +758,8 @@ guestfs___accept_from_daemon (guestfs_h *g)
return -1;
}
- if (g->fd[1] >= 0 && FD_ISSET (g->fd[1], &rset2)) {
- if (read_log_message_or_eof (g, g->fd[1], 1) == -1)
+ if (g->fd >= 0 && FD_ISSET (g->fd, &rset2)) {
+ if (read_log_message_or_eof (g, g->fd, 1) == -1)
return -1;
}
if (FD_ISSET (g->sock, &rset2)) {
--
1.8.1.4
Richard W.M. Jones
2013-Mar-07 11:44 UTC
[Libguestfs] [PATCH 2/4] launch: appliance: Set FD_CLOEXEC flag on console socket.
From: "Richard W.M. Jones" <rjones at redhat.com>
The earlier (pipe-based) code never set this flag, but that was a bug,
potentially allowing the file descriptor to be leaked to subprocesses.
Set the FD_CLOEXEC flag, but also ensure it is cleared in the child
process just before qemu is exec'd (otherwise qemu would not have a
console).
---
src/launch-appliance.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/launch-appliance.c b/src/launch-appliance.c
index 4016d61..30c139e 100644
--- a/src/launch-appliance.c
+++ b/src/launch-appliance.c
@@ -33,6 +33,7 @@
#include <pcre.h>
+#include "cloexec.h"
#include "ignore-value.h"
#include "guestfs.h"
@@ -235,7 +236,7 @@ launch_appliance (guestfs_h *g, const char *arg)
}
if (!g->direct) {
- if (socketpair (AF_LOCAL, SOCK_STREAM, 0, sv) == -1) {
+ if (socketpair (AF_LOCAL, SOCK_STREAM|SOCK_CLOEXEC, 0, sv) == -1) {
perrorf (g, "socketpair");
goto cleanup0;
}
@@ -484,6 +485,12 @@ launch_appliance (guestfs_h *g, const char *arg)
close (1);
close (sv[0]);
+ /* We set the FD_CLOEXEC flag on the socket above, but now (in
+ * the child) it's safe to unset this flag so qemu can use the
+ * socket.
+ */
+ set_cloexec_flag (sv[1], 0);
+
/* Stdin. */
if (dup (sv[1]) == -1) {
dup_failed:
--
1.8.1.4
Richard W.M. Jones
2013-Mar-07 11:44 UTC
[Libguestfs] [PATCH 3/4] lib: Rename g->fd to console_sock and g->sock to daemon_sock.
From: "Richard W.M. Jones" <rjones at redhat.com>
This just renames the fields in the handle to more descriptive names.
There is no functional change.
---
src/guestfs-internal.h | 4 +--
src/handle.c | 16 +++++------
src/launch-appliance.c | 30 ++++++++++-----------
src/launch-libvirt.c | 32 +++++++++++-----------
src/launch-unix.c | 14 +++++-----
src/proto.c | 72 +++++++++++++++++++++++++-------------------------
6 files changed, 84 insertions(+), 84 deletions(-)
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 4c632e8..c63ea40 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -263,8 +263,8 @@ struct guestfs_h
int unique;
/*** Protocol. ***/
- int fd; /* Stdin/stdout of qemu. */
- int sock; /* Daemon communications socket. */
+ int console_sock; /* Appliance console (for debug info). */
+ int daemon_sock; /* Daemon communications socket. */
int msg_next_serial;
#if HAVE_FUSE
diff --git a/src/handle.c b/src/handle.c
index cefe385..47a2246 100644
--- a/src/handle.c
+++ b/src/handle.c
@@ -91,8 +91,8 @@ guestfs_create_flags (unsigned flags, ...)
g->state = CONFIG;
- g->fd = -1;
- g->sock = -1;
+ g->console_sock = -1;
+ g->daemon_sock = -1;
guestfs___init_error_handler (g);
g->abort_cb = abort;
@@ -366,12 +366,12 @@ shutdown_backend (guestfs_h *g, int check_for_errors)
}
/* Close sockets. */
- if (g->fd >= 0)
- close (g->fd);
- if (g->sock >= 0)
- close (g->sock);
- g->fd = -1;
- g->sock = -1;
+ if (g->console_sock >= 0)
+ close (g->console_sock);
+ if (g->daemon_sock >= 0)
+ close (g->daemon_sock);
+ g->console_sock = -1;
+ g->daemon_sock = -1;
if (g->attach_ops->shutdown (g, check_for_errors) == -1)
ret = -1;
diff --git a/src/launch-appliance.c b/src/launch-appliance.c
index 30c139e..99bc541 100644
--- a/src/launch-appliance.c
+++ b/src/launch-appliance.c
@@ -210,13 +210,13 @@ launch_appliance (guestfs_h *g, const char *arg)
snprintf (guestfsd_sock, sizeof guestfsd_sock, "%s/guestfsd.sock",
g->tmpdir);
unlink (guestfsd_sock);
- g->sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
- if (g->sock == -1) {
+ g->daemon_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
+ if (g->daemon_sock == -1) {
perrorf (g, "socket");
goto cleanup0;
}
- if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) {
+ if (fcntl (g->daemon_sock, F_SETFL, O_NONBLOCK) == -1) {
perrorf (g, "fcntl");
goto cleanup0;
}
@@ -225,12 +225,12 @@ launch_appliance (guestfs_h *g, const char *arg)
strncpy (addr.sun_path, guestfsd_sock, UNIX_PATH_MAX);
addr.sun_path[UNIX_PATH_MAX-1] = '\0';
- if (bind (g->sock, &addr, sizeof addr) == -1) {
+ if (bind (g->daemon_sock, &addr, sizeof addr) == -1) {
perrorf (g, "bind");
goto cleanup0;
}
- if (listen (g->sock, 1) == -1) {
+ if (listen (g->daemon_sock, 1) == -1) {
perrorf (g, "listen");
goto cleanup0;
}
@@ -611,7 +611,7 @@ launch_appliance (guestfs_h *g, const char *arg)
goto cleanup1;
}
- g->fd = sv[0]; /* stdin of child */
+ g->console_sock = sv[0]; /* stdin of child */
sv[0] = -1;
}
@@ -632,15 +632,15 @@ launch_appliance (guestfs_h *g, const char *arg)
*/
/* Close the listening socket. */
- if (close (g->sock) != 0) {
+ if (close (g->daemon_sock) != 0) {
perrorf (g, "close: listening socket");
close (r);
- g->sock = -1;
+ g->daemon_sock = -1;
goto cleanup1;
}
- g->sock = r; /* This is the accepted data socket. */
+ g->daemon_sock = r; /* This is the accepted data socket. */
- if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) {
+ if (fcntl (g->daemon_sock, F_SETFL, O_NONBLOCK) == -1) {
perrorf (g, "fcntl");
goto cleanup1;
}
@@ -686,16 +686,16 @@ launch_appliance (guestfs_h *g, const char *arg)
if (g->app.recoverypid > 0) kill (g->app.recoverypid, 9);
if (g->app.pid > 0) waitpid (g->app.pid, NULL, 0);
if (g->app.recoverypid > 0) waitpid (g->app.recoverypid, NULL, 0);
- if (g->fd >= 0) close (g->fd);
- g->fd = -1;
+ if (g->console_sock >= 0) close (g->console_sock);
+ g->console_sock = -1;
g->app.pid = 0;
g->app.recoverypid = 0;
memset (&g->launch_t, 0, sizeof g->launch_t);
cleanup0:
- if (g->sock >= 0) {
- close (g->sock);
- g->sock = -1;
+ if (g->daemon_sock >= 0) {
+ close (g->daemon_sock);
+ g->daemon_sock = -1;
}
g->state = CONFIG;
return -1;
diff --git a/src/launch-libvirt.c b/src/launch-libvirt.c
index caad5d4..d771392 100644
--- a/src/launch-libvirt.c
+++ b/src/launch-libvirt.c
@@ -258,13 +258,13 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
set_socket_create_context (g);
- g->sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
- if (g->sock == -1) {
+ g->daemon_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
+ if (g->daemon_sock == -1) {
perrorf (g, "socket");
goto cleanup;
}
- if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) {
+ if (fcntl (g->daemon_sock, F_SETFL, O_NONBLOCK) == -1) {
perrorf (g, "fcntl");
goto cleanup;
}
@@ -272,12 +272,12 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
addr.sun_family = AF_UNIX;
memcpy (addr.sun_path, params.guestfsd_sock, UNIX_PATH_MAX);
- if (bind (g->sock, &addr, sizeof addr) == -1) {
+ if (bind (g->daemon_sock, &addr, sizeof addr) == -1) {
perrorf (g, "bind");
goto cleanup;
}
- if (listen (g->sock, 1) == -1) {
+ if (listen (g->daemon_sock, 1) == -1) {
perrorf (g, "listen");
goto cleanup;
}
@@ -399,7 +399,7 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
goto cleanup;
}
console = -1;
- g->fd = r; /* This is the accepted console socket. */
+ g->console_sock = r; /* This is the accepted console socket. */
/* Wait for libvirt domain to start and to connect back to us via
* virtio-serial and send the GUESTFS_LAUNCH_FLAG message.
@@ -416,15 +416,15 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
*/
/* Close the listening socket. */
- if (close (g->sock) == -1) {
+ if (close (g->daemon_sock) == -1) {
perrorf (g, "close: listening socket");
close (r);
- g->sock = -1;
+ g->daemon_sock = -1;
goto cleanup;
}
- g->sock = r; /* This is the accepted data socket. */
+ g->daemon_sock = r; /* This is the accepted data socket. */
- if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) {
+ if (fcntl (g->daemon_sock, F_SETFL, O_NONBLOCK) == -1) {
perrorf (g, "fcntl");
goto cleanup;
}
@@ -475,13 +475,13 @@ launch_libvirt (guestfs_h *g, const char *libvirt_uri)
if (console >= 0)
close (console);
- if (g->fd >= 0) {
- close (g->fd);
- g->fd = -1;
+ if (g->console_sock >= 0) {
+ close (g->console_sock);
+ g->console_sock = -1;
}
- if (g->sock >= 0) {
- close (g->sock);
- g->sock = -1;
+ if (g->daemon_sock >= 0) {
+ close (g->daemon_sock);
+ g->daemon_sock = -1;
}
if (dom) {
diff --git a/src/launch-unix.c b/src/launch-unix.c
index a7a3ae2..69de2f7 100644
--- a/src/launch-unix.c
+++ b/src/launch-unix.c
@@ -49,13 +49,13 @@ launch_unix (guestfs_h *g, const char *sockpath)
/* Set this to nothing so we don't try to read from a random file
* descriptor.
*/
- g->fd = -1;
+ g->console_sock = -1;
if (g->verbose)
guestfs___print_timestamped_message (g, "connecting to %s",
sockpath);
- g->sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
- if (g->sock == -1) {
+ g->daemon_sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
+ if (g->daemon_sock == -1) {
perrorf (g, "socket");
return -1;
}
@@ -66,12 +66,12 @@ launch_unix (guestfs_h *g, const char *sockpath)
g->state = LAUNCHING;
- if (connect (g->sock, &addr, sizeof addr) == -1) {
+ if (connect (g->daemon_sock, &addr, sizeof addr) == -1) {
perrorf (g, "bind");
goto cleanup;
}
- if (fcntl (g->sock, F_SETFL, O_NONBLOCK) == -1) {
+ if (fcntl (g->daemon_sock, F_SETFL, O_NONBLOCK) == -1) {
perrorf (g, "fcntl");
goto cleanup;
}
@@ -97,14 +97,14 @@ launch_unix (guestfs_h *g, const char *sockpath)
return 0;
cleanup:
- close (g->sock);
+ close (g->daemon_sock);
return -1;
}
static int
shutdown_unix (guestfs_h *g, int check_for_errors)
{
- /* Merely closing g->sock is sufficient and that is already done
+ /* Merely closing g->daemon_sock is sufficient and that is already done
* in the calling code.
*/
return 0;
diff --git a/src/proto.c b/src/proto.c
index 57cb609..569b805 100644
--- a/src/proto.c
+++ b/src/proto.c
@@ -161,10 +161,10 @@ child_cleanup (guestfs_h *g)
debug (g, "child_cleanup: %p: child process died", g);
g->attach_ops->shutdown (g, 0);
- if (g->fd >= 0) close (g->fd);
- close (g->sock);
- g->fd = -1;
- g->sock = -1;
+ if (g->console_sock >= 0) close (g->console_sock);
+ close (g->daemon_sock);
+ g->console_sock = -1;
+ g->daemon_sock = -1;
memset (&g->launch_t, 0, sizeof g->launch_t);
guestfs___free_drives (g);
g->state = CONFIG;
@@ -383,12 +383,12 @@ send_to_daemon (guestfs_h *g, const void *v_buf, size_t n)
FD_ZERO (&rset);
FD_ZERO (&wset);
- if (g->fd >= 0) /* Read qemu stdout for log messages
& EOF. */
- FD_SET (g->fd, &rset);
- FD_SET (g->sock, &rset); /* Read socket for cancellation &
EOF. */
- FD_SET (g->sock, &wset); /* Write to socket to send the data. */
+ if (g->console_sock >= 0) /* Read qemu stdout for log messages &
EOF. */
+ FD_SET (g->console_sock, &rset);
+ FD_SET (g->daemon_sock, &rset); /* Read socket for cancellation &
EOF. */
+ FD_SET (g->daemon_sock, &wset); /* Write to socket to send the data.
*/
- int max_fd = MAX (g->sock, g->fd);
+ int max_fd = MAX (g->daemon_sock, g->console_sock);
while (n > 0) {
rset2 = rset;
@@ -401,12 +401,12 @@ send_to_daemon (guestfs_h *g, const void *v_buf, size_t n)
return -1;
}
- if (g->fd >= 0 && FD_ISSET (g->fd, &rset2)) {
- if (read_log_message_or_eof (g, g->fd, 0) == -1)
+ if (g->console_sock >= 0 && FD_ISSET (g->console_sock,
&rset2)) {
+ if (read_log_message_or_eof (g, g->console_sock, 0) == -1)
return -1;
}
- if (FD_ISSET (g->sock, &rset2)) {
- r = check_for_daemon_cancellation_or_eof (g, g->sock);
+ if (FD_ISSET (g->daemon_sock, &rset2)) {
+ r = check_for_daemon_cancellation_or_eof (g, g->daemon_sock);
if (r == -1)
return r;
if (r == -2) {
@@ -414,15 +414,15 @@ send_to_daemon (guestfs_h *g, const void *v_buf, size_t n)
* synchronization we must write out the remainder of the
* write buffer before we return (RHBZ#576879).
*/
- if (xwrite (g->sock, buf, n) == -1) {
+ if (xwrite (g->daemon_sock, buf, n) == -1) {
perrorf (g, "write");
return -1;
}
return -2; /* cancelled */
}
}
- if (FD_ISSET (g->sock, &wset2)) {
- r = write (g->sock, buf, n);
+ if (FD_ISSET (g->daemon_sock, &wset2)) {
+ r = write (g->daemon_sock, buf, n);
if (r == -1) {
if (errno == EINTR || errno == EAGAIN)
continue;
@@ -507,18 +507,18 @@ recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void
**buf_rtn)
* socket connection just before this function is called, so just
* return an error if this happens.
*/
- if (g->sock == -1) {
+ if (g->daemon_sock == -1) {
unexpected_closed_connection_from_daemon_error (g);
return -1;
}
FD_ZERO (&rset);
- if (g->fd >= 0) /* Read qemu stdout for log messages
& EOF. */
- FD_SET (g->fd, &rset);
- FD_SET (g->sock, &rset); /* Read socket for data & EOF. */
+ if (g->console_sock >= 0) /* Read qemu stdout for log messages &
EOF. */
+ FD_SET (g->console_sock, &rset);
+ FD_SET (g->daemon_sock, &rset); /* Read socket for data & EOF. */
- max_fd = MAX (g->sock, g->fd);
+ max_fd = MAX (g->daemon_sock, g->console_sock);
*size_rtn = 0;
*buf_rtn = NULL;
@@ -546,16 +546,16 @@ recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void
**buf_rtn)
return -1;
}
- if (g->fd >= 0 && FD_ISSET (g->fd, &rset2)) {
- if (read_log_message_or_eof (g, g->fd, 0) == -1) {
+ if (g->console_sock >= 0 && FD_ISSET (g->console_sock,
&rset2)) {
+ if (read_log_message_or_eof (g, g->console_sock, 0) == -1) {
free (*buf_rtn);
*buf_rtn = NULL;
return -1;
}
}
- if (FD_ISSET (g->sock, &rset2)) {
+ if (FD_ISSET (g->daemon_sock, &rset2)) {
if (nr < 0) { /* Have we read the message length word yet? */
- r = read (g->sock, lenbuf+nr+4, -nr);
+ r = read (g->daemon_sock, lenbuf+nr+4, -nr);
if (r == -1) {
if (errno == EINTR || errno == EAGAIN)
continue;
@@ -623,7 +623,7 @@ recv_from_daemon (guestfs_h *g, uint32_t *size_rtn, void
**buf_rtn)
size_t sizetoread = message_size - nr;
if (sizetoread > BUFSIZ) sizetoread = BUFSIZ;
- r = read (g->sock, (char *) (*buf_rtn) + nr, sizetoread);
+ r = read (g->daemon_sock, (char *) (*buf_rtn) + nr, sizetoread);
if (r == -1) {
if (errno == EINTR || errno == EAGAIN)
continue;
@@ -712,7 +712,7 @@ guestfs___recv_from_daemon (guestfs_h *g, uint32_t
*size_rtn, void **buf_rtn)
return 0;
}
-/* This is very much like recv_from_daemon above, but g->sock is
+/* This is very much like recv_from_daemon above, but g->daemon_sock is
* a listening socket and we are accepting a new connection on
* that socket instead of reading anything. Returns the newly
* accepted socket.
@@ -726,11 +726,11 @@ guestfs___accept_from_daemon (guestfs_h *g)
FD_ZERO (&rset);
- if (g->fd >= 0) /* Read qemu stdout for log messages
& EOF. */
- FD_SET (g->fd, &rset);
- FD_SET (g->sock, &rset); /* Read socket for accept. */
+ if (g->console_sock >= 0) /* Read qemu stdout for log messages &
EOF. */
+ FD_SET (g->console_sock, &rset);
+ FD_SET (g->daemon_sock, &rset); /* Read socket for accept. */
- int max_fd = MAX (g->sock, g->fd);
+ int max_fd = MAX (g->daemon_sock, g->console_sock);
int sock = -1;
while (sock == -1) {
@@ -758,12 +758,12 @@ guestfs___accept_from_daemon (guestfs_h *g)
return -1;
}
- if (g->fd >= 0 && FD_ISSET (g->fd, &rset2)) {
- if (read_log_message_or_eof (g, g->fd, 1) == -1)
+ if (g->console_sock >= 0 && FD_ISSET (g->console_sock,
&rset2)) {
+ if (read_log_message_or_eof (g, g->console_sock, 1) == -1)
return -1;
}
- if (FD_ISSET (g->sock, &rset2)) {
- sock = accept4 (g->sock, NULL, NULL, SOCK_CLOEXEC);
+ if (FD_ISSET (g->daemon_sock, &rset2)) {
+ sock = accept4 (g->daemon_sock, NULL, NULL, SOCK_CLOEXEC);
if (sock == -1) {
if (errno == EINTR || errno == EAGAIN)
continue;
@@ -1154,7 +1154,7 @@ guestfs___recv_file (guestfs_h *g, const char *filename)
xdr_uint32_t (&xdr, &flag);
xdr_destroy (&xdr);
- if (xwrite (g->sock, fbuf, sizeof fbuf) == -1) {
+ if (xwrite (g->daemon_sock, fbuf, sizeof fbuf) == -1) {
perrorf (g, _("write to daemon socket"));
return -1;
}
--
1.8.1.4
Richard W.M. Jones
2013-Mar-07 11:44 UTC
[Libguestfs] [PATCH 4/4] launch: unix: Set g->daemon_sock = -1 after closing it.
From: "Richard W.M. Jones" <rjones at redhat.com> This ensures we don't accidentally use the closed fd. --- src/launch-unix.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/launch-unix.c b/src/launch-unix.c index 69de2f7..db49a3c 100644 --- a/src/launch-unix.c +++ b/src/launch-unix.c @@ -98,6 +98,7 @@ launch_unix (guestfs_h *g, const char *sockpath) cleanup: close (g->daemon_sock); + g->daemon_sock = -1; return -1; } -- 1.8.1.4
Apparently Analagous Threads
- [PATCH 0/3] protocol: Abstract out socket operations.
- Libguestfs (1.22.6) driver/changes for mingw/win32
- [PATCH] lib: command: switch from select() to poll()
- Re: [PATCH] lib: command: switch from select() to poll()
- [PATCH v2 0/4] Experimental User-Mode Linux backend.