X-Git-Url: http://git.annexia.org/?a=blobdiff_plain;ds=sidebyside;f=src%2Fguestfs.c;h=05fb113b864648a7530d37d46652cb879b6d2d64;hb=a30f66436eac79b9e532e448882d03375c0d5e78;hp=4dc8dec7c98c46f7eb338b270aa1fd18c400b54d;hpb=5f8c46db05966a6e31b696d629fd3f74e4fb6c71;p=libguestfs.git diff --git a/src/guestfs.c b/src/guestfs.c index 4dc8dec..05fb113 100644 --- a/src/guestfs.c +++ b/src/guestfs.c @@ -143,7 +143,8 @@ struct guestfs_h int fd[2]; /* Stdin/stdout of qemu. */ int sock; /* Daemon communications socket. */ - int pid; /* Qemu PID. */ + pid_t pid; /* Qemu PID. */ + pid_t recoverypid; /* Recovery process PID. */ time_t start_t; /* The time when we started qemu. */ int stdout_watch; /* Watches qemu stdout for log messages. */ @@ -729,9 +730,9 @@ guestfs_launch (guestfs_h *g) /* Want to give userspace some room, so: */ memsize += 128; -#if AC_SIZEOF_LONG == 8 +#if SIZEOF_LONG == 8 /* On 64 bit, assume some overhead. */ - memsize += 32; + memsize += 64; #endif } else memsize = 512; @@ -842,6 +843,40 @@ guestfs_launch (guestfs_h *g) /* Parent (library). */ g->pid = r; + /* Fork the recovery process off which will kill qemu if the parent + * process fails to do so (eg. if the parent segfaults). + */ + r = fork (); + if (r == 0) { + pid_t qemu_pid = g->pid; + pid_t parent_pid = getppid (); + + /* Writing to argv is hideously complicated and error prone. See: + * http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/misc/ps_status.c?rev=1.33.2.1;content-type=text%2Fplain + */ + + /* Loop around waiting for one or both of the other processes to + * disappear. It's fair to say this is very hairy. The PIDs that + * we are looking at might be reused by another process. We are + * effectively polling. Is the cure worse than the disease? + */ + for (;;) { + if (kill (qemu_pid, 0) == -1) /* qemu's gone away, we aren't needed */ + _exit (0); + if (kill (parent_pid, 0) == -1) { + /* Parent's gone away, qemu still around, so kill qemu. */ + kill (qemu_pid, 9); + _exit (0); + } + sleep (2); + } + } + + /* Don't worry, if the fork failed, this will be -1. The recovery + * process isn't essential. + */ + g->recoverypid = r; + /* Start the clock ... */ time (&g->start_t); @@ -938,11 +973,14 @@ guestfs_launch (guestfs_h *g) close (wfd[1]); close (rfd[0]); kill (g->pid, 9); + if (g->recoverypid > 0) kill (g->recoverypid, 9); waitpid (g->pid, NULL, 0); + if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0); g->fd[0] = -1; g->fd[1] = -1; g->sock = -1; g->pid = 0; + g->recoverypid = 0; g->start_t = 0; g->stdout_watch = -1; g->sock_watch = -1; @@ -1018,6 +1056,7 @@ guestfs_kill_subprocess (guestfs_h *g) fprintf (stderr, "sending SIGTERM to process %d\n", g->pid); kill (g->pid, SIGTERM); + if (g->recoverypid > 0) kill (g->recoverypid, 9); return 0; } @@ -1135,7 +1174,9 @@ stdout_event (struct guestfs_main_loop *ml, guestfs_h *g, void *data, if (g->verbose) fprintf (stderr, "stdout_event: %p: child process died\n", g); /*kill (g->pid, SIGTERM);*/ + if (g->recoverypid > 0) kill (g->recoverypid, 9); waitpid (g->pid, NULL, 0); + if (g->recoverypid > 0) waitpid (g->recoverypid, NULL, 0); if (g->stdout_watch >= 0) g->main_loop->remove_handle (g->main_loop, g, g->stdout_watch); if (g->sock_watch >= 0) @@ -1147,6 +1188,7 @@ stdout_event (struct guestfs_main_loop *ml, guestfs_h *g, void *data, g->fd[1] = -1; g->sock = -1; g->pid = 0; + g->recoverypid = 0; g->start_t = 0; g->stdout_watch = -1; g->sock_watch = -1; @@ -1492,7 +1534,6 @@ int guestfs__send_sync (guestfs_h *g, int proc_nr, xdrproc_t xdrp, char *args) { - char buffer[GUESTFS_MESSAGE_MAX]; struct guestfs_message_header hdr; XDR xdr; unsigned len; @@ -1505,6 +1546,23 @@ guestfs__send_sync (guestfs_h *g, int proc_nr, return -1; } + /* This is probably an internal error. Or perhaps we should just + * free the buffer anyway? + */ + if (g->msg_out != NULL) { + error (g, "guestfs__send_sync: msg_out should be NULL"); + return -1; + } + + /* We have to allocate this message buffer on the heap because + * it is quite large (although will be mostly unused). We + * can't allocate it on the stack because in some environments + * we have quite limited stack space available, notably when + * running in the JVM. + */ + g->msg_out = safe_malloc (g, GUESTFS_MESSAGE_MAX + 4); + xdrmem_create (&xdr, g->msg_out + 4, GUESTFS_MESSAGE_MAX, XDR_ENCODE); + /* Serialize the header. */ hdr.prog = GUESTFS_PROGRAM; hdr.vers = GUESTFS_PROTOCOL_VERSION; @@ -1513,10 +1571,9 @@ guestfs__send_sync (guestfs_h *g, int proc_nr, hdr.serial = serial; hdr.status = GUESTFS_STATUS_OK; - xdrmem_create (&xdr, buffer, sizeof buffer, XDR_ENCODE); if (!xdr_guestfs_message_header (&xdr, &hdr)) { error (g, "xdr_guestfs_message_header failed"); - return -1; + goto cleanup1; } /* Serialize the args. If any, because some message types @@ -1525,24 +1582,23 @@ guestfs__send_sync (guestfs_h *g, int proc_nr, if (xdrp) { if (!(*xdrp) (&xdr, args)) { error (g, "dispatch failed to marshal args"); - return -1; + goto cleanup1; } } + /* Get the actual length of the message, resize the buffer to match + * the actual length, and write the length word at the beginning. + */ len = xdr_getpos (&xdr); xdr_destroy (&xdr); - /* Allocate the outgoing message buffer. */ - g->msg_out = safe_malloc (g, len + 4); - + g->msg_out = safe_realloc (g, g->msg_out, len + 4); g->msg_out_size = len + 4; g->msg_out_pos = 0; xdrmem_create (&xdr, g->msg_out, 4, XDR_ENCODE); xdr_uint32_t (&xdr, &len); - memcpy (g->msg_out + 4, buffer, len); - if (guestfs__switch_to_sending (g) == -1) goto cleanup1; @@ -1653,10 +1709,9 @@ send_file_complete_sync (guestfs_h *g) static int check_for_daemon_cancellation (guestfs_h *g); static int -send_file_chunk_sync (guestfs_h *g, int cancel, const char *buf, size_t len) +send_file_chunk_sync (guestfs_h *g, int cancel, const char *buf, size_t buflen) { - char data[GUESTFS_MAX_CHUNK_SIZE + 48]; - unsigned datalen; + unsigned len; int sent; guestfs_chunk chunk; XDR xdr; @@ -1667,6 +1722,14 @@ send_file_chunk_sync (guestfs_h *g, int cancel, const char *buf, size_t len) return -1; } + /* This is probably an internal error. Or perhaps we should just + * free the buffer anyway? + */ + if (g->msg_out != NULL) { + error (g, "guestfs__send_sync: msg_out should be NULL"); + return -1; + } + /* Did the daemon send a cancellation message? */ if (check_for_daemon_cancellation (g)) { if (g->verbose) @@ -1674,35 +1737,34 @@ send_file_chunk_sync (guestfs_h *g, int cancel, const char *buf, size_t len) return -2; } + /* Allocate the chunk buffer. Don't use the stack to avoid + * excessive stack usage and unnecessary copies. + */ + g->msg_out = safe_malloc (g, GUESTFS_MAX_CHUNK_SIZE + 4 + 48); + xdrmem_create (&xdr, g->msg_out + 4, GUESTFS_MAX_CHUNK_SIZE + 48, XDR_ENCODE); + /* Serialize the chunk. */ chunk.cancel = cancel; - chunk.data.data_len = len; + chunk.data.data_len = buflen; chunk.data.data_val = (char *) buf; - if (g->verbose) - fprintf (stderr, - "library sending chunk cancel = %d, len = %zu, buf = %p\n", - cancel, len, buf); - - xdrmem_create (&xdr, data, sizeof data, XDR_ENCODE); if (!xdr_guestfs_chunk (&xdr, &chunk)) { - error (g, "xdr_guestfs_chunk failed (buf = %p, len = %zu)", buf, len); + error (g, "xdr_guestfs_chunk failed (buf = %p, buflen = %zu)", + buf, buflen); xdr_destroy (&xdr); - return -1; + goto cleanup1; } - datalen = xdr_getpos (&xdr); + len = xdr_getpos (&xdr); xdr_destroy (&xdr); - /* Allocate outgoing message buffer. */ - g->msg_out = safe_malloc (g, datalen + 4); - g->msg_out_size = datalen + 4; + /* Reduce the size of the outgoing message buffer to the real length. */ + g->msg_out = safe_realloc (g, g->msg_out, len + 4); + g->msg_out_size = len + 4; g->msg_out_pos = 0; xdrmem_create (&xdr, g->msg_out, 4, XDR_ENCODE); - xdr_uint32_t (&xdr, &datalen); - - memcpy (g->msg_out + 4, data, datalen); + xdr_uint32_t (&xdr, &len); if (guestfs__switch_to_sending (g) == -1) goto cleanup1;