Hi Jim, comments inline Jim Meyering wrote:
diff --git a/proxy/libvirt_proxy.c b/proxy/libvirt_proxy.c index d96d3db..a22ba6c 100644 --- a/proxy/libvirt_proxy.c +++ b/proxy/libvirt_proxy.c @@ -2,7 +2,7 @@ * proxy_svr.c: root suid proxy server for Xen access to APIs with no * side effects from unauthenticated clients. * - * Copyright (C) 2006, 2007 Red Hat, Inc. + * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -26,6 +26,7 @@ #include "internal.h"
#include "proxy_internal.h" +#include "util.h" #include "xen_internal.h" #include "xend_internal.h" #include "xs_internal.h" @@ -317,19 +318,12 @@ proxyWriteClientSocket(int nr, virProxyPacketPtr req) { return(-1); }
-retry: - ret = write(pollInfos[nr].fd, (char *) req, req->len); + ret = safewrite(pollInfos[nr].fd, (char *) req, req->len); if (ret < 0) {
Should this check (ret == req->len) instead? safewrite() will return an error if write() returns an error, regardless of how many bytes are written, but it's still possible for it to return less than requested if write() returns 0 (eof?). The behavior of safewrite could be adjusted to return errors in that case.
- if (errno == EINTR) { - if (debug > 0) - fprintf(stderr, "write socket %d to client %d interrupted\n", - pollInfos[nr].fd, nr); - goto retry; - } fprintf(stderr, "write %d bytes to socket %d from client %d failed\n", req->len, pollInfos[nr].fd, nr); - proxyCloseClientSocket(nr); - return(-1); + proxyCloseClientSocket(nr); + return(-1); } if (ret == 0) { if (debug) diff --git a/qemud/qemud.c b/qemud/qemud.c index 3a5e44c..269e9fe 100644 --- a/qemud/qemud.c +++ b/qemud/qemud.c @@ -118,7 +118,7 @@ static void sig_handler(int sig) { return;
origerrno = errno; - r = write(sigwrite, &sigc, 1); + r = safewrite(sigwrite, &sigc, 1);
write() shouldn't do a partial write for size 1, but this is necessary anyway to help with the EINTR case. Might want to add that benefit to the log message, it's not just short-writes this protects against.
if (r == -1) {
if (r != 1)?
sig_errors++; sig_lasterrno = errno; @@ -1360,11 +1360,9 @@ static int qemudClientWriteBuf(struct qemud_server *server, const char *data, int len) { int ret; if (!client->tlssession) { - if ((ret = write(client->fd, data, len)) == -1) { - if (errno != EAGAIN) { - qemudLog (QEMUD_ERR, _("write: %s"), strerror (errno)); - qemudDispatchClientFailure(server, client); - } + if ((ret = safewrite(client->fd, data, len)) == -1) { + qemudLog (QEMUD_ERR, _("write: %s"), strerror (errno)); + qemudDispatchClientFailure(server, client);
ret != len?
return -1; } } else { diff --git a/src/conf.c b/src/conf.c index e0ecdea..53ea993 100644 --- a/src/conf.c +++ b/src/conf.c @@ -904,7 +904,7 @@ __virConfWriteFile(const char *filename, virConfPtr conf) goto error; }
- ret = write(fd, buf->content, buf->use); + ret = safewrite(fd, buf->content, buf->use); close(fd); if (ret != (int) buf->use) { virConfError(NULL, VIR_ERR_WRITE_FAILED, _("failed to save content"), 0); diff --git a/src/console.c b/src/console.c index 02a9c7f..1c6cba0 100644 --- a/src/console.c +++ b/src/console.c @@ -1,7 +1,7 @@ /* * console.c: A dumb serial console client * - * Copyright (C) 2007 Red Hat, Inc. + * Copyright (C) 2007, 2008 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -37,6 +37,7 @@
#include "console.h" #include "internal.h" +#include "util.h"
/* ie Ctrl-] as per telnet */ #define CTRL_CLOSE_BRACKET '\35' @@ -161,7 +162,8 @@ int vshRunConsole(const char *tty) {
while (sent < got) { int done; - if ((done = write(destfd, buf + sent, got - sent)) <= 0) { + if ((done = safewrite(destfd, buf + sent, got - sent)) + <= 0) {
!= (got - sent)?
fprintf(stderr, _("failure writing output: %s\n"), strerror(errno)); goto cleanup; diff --git a/src/proxy_internal.c b/src/proxy_internal.c index bc94763..c3e50c6 100644 --- a/src/proxy_internal.c +++ b/src/proxy_internal.c @@ -1,7 +1,7 @@ /* * proxy_client.c: client side of the communication with the libvirt proxy. * - * Copyright (C) 2006 Red Hat, Inc. + * Copyright (C) 2006, 2008 Red Hat, Inc. * * See COPYING.LIB for the License of this software * @@ -26,6 +26,7 @@ #include "internal.h" #include "driver.h" #include "proxy_internal.h" +#include "util.h" #include "xen_unified.h"
#define STANDALONE @@ -345,17 +346,10 @@ virProxyWriteClientSocket(int fd, const char *data, int len) { if ((fd < 0) || (data == NULL) || (len < 0)) return(-1);
-retry: - ret = write(fd, data, len); + ret = safewrite(fd, data, len); if (ret < 0) {
ret != len? Or at least (ret <= 0).
- if (errno == EINTR) { - if (debug > 0) - fprintf(stderr, "write socket %d, %d bytes interrupted\n", - fd, len); - goto retry; - } fprintf(stderr, _("Failed to write to socket %d\n"), fd); - return(-1); + return(-1); } if (debug) fprintf(stderr, "wrote %d bytes to socket %d\n", diff --git a/src/qemu_conf.c b/src/qemu_conf.c index e39c7bc..ff6a92e 100644 --- a/src/qemu_conf.c +++ b/src/qemu_conf.c @@ -1866,7 +1866,7 @@ static int qemudSaveConfig(virConnectPtr conn, }
towrite = strlen(xml); - if (write(fd, xml, towrite) != towrite) { + if (safewrite(fd, xml, towrite) < 0) {
Stick with "!= towrite" or at least "<= 0"
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "cannot write config file %s: %s", vm->configFile, strerror(errno)); @@ -2089,7 +2089,7 @@ static int qemudSaveNetworkConfig(virConnectPtr conn, }
towrite = strlen(xml); - if (write(fd, xml, towrite) != towrite) { + if (safewrite(fd, xml, towrite) < 0) {
ditto
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, "cannot write config file %s: %s", network->configFile, strerror(errno)); diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 15cd52c..85d042f 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -539,11 +539,9 @@ static int qemudWaitForMonitor(virConnectPtr conn, "console");
buf[sizeof(buf)-1] = '\0'; - retry: - if (write(vm->logfile, buf, strlen(buf)) < 0) { + + if (safewrite(vm->logfile, buf, strlen(buf)) < 0) {
!= strlen(buf)
/* Log, but ignore failures to write logfile for VM */ - if (errno == EINTR) - goto retry; qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s"), strerror(errno)); } @@ -656,15 +654,15 @@ static int qemudStartVMDaemon(virConnectPtr conn,
tmp = argv; while (*tmp) { - if (write(vm->logfile, *tmp, strlen(*tmp)) < 0) + if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
!= strlen(*tmp)
qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s"), errno, strerror(errno)); - if (write(vm->logfile, " ", 1) < 0) + if (safewrite(vm->logfile, " ", 1) < 0)
!= 1
qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s"), errno, strerror(errno)); tmp++; } - if (write(vm->logfile, "\n", 1) < 0) + if (safewrite(vm->logfile, "\n", 1) < 0)
!= 1
qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s"), errno, strerror(errno));
@@ -733,11 +731,8 @@ static int qemudVMData(struct qemud_driver *driver ATTRIBUTE_UNUSED, } buf[ret] = '\0';
- retry: - if (write(vm->logfile, buf, ret) < 0) { + if (safewrite(vm->logfile, buf, ret) < 0) {
!= ret
/* Log, but ignore failures to write logfile for VM */ - if (errno == EINTR) - goto retry; qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s"), strerror(errno)); } @@ -1113,7 +1108,7 @@ qemudEnableIpForwarding(void) if ((fd = open(PROC_IP_FORWARD, O_WRONLY|O_TRUNC)) == -1) return 0;
- if (write(fd, "1\n", 2) < 0) + if (safewrite(fd, "1\n", 2) < 0)
!= 2
ret = 0;
close (fd); diff --git a/src/test.c b/src/test.c index 003d6b7..346774b 100644 --- a/src/test.c +++ b/src/test.c @@ -42,6 +42,7 @@ #include "test.h" #include "xml.h" #include "buf.h" +#include "util.h" #include "uuid.h"
/* Flags that determine the action to take on a shutdown or crash of a domain @@ -1293,19 +1294,19 @@ static int testDomainSave(virDomainPtr domain, return (-1); } len = strlen(xml); - if (write(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) != sizeof(TEST_SAVE_MAGIC)) { + if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) {
keep the != sizeof(TEST_SAVE_MAGIC)
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "cannot write header"); close(fd); return (-1); } - if (write(fd, (char*)&len, sizeof(len)) != sizeof(len)) { + if (safewrite(fd, (char*)&len, sizeof(len)) < 0) {
keep the != sizeof(len)
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "cannot write metadata length"); close(fd); return (-1); } - if (write(fd, xml, len) != len) { + if (safewrite(fd, xml, len) < 0) {
keep the != len
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "cannot write metadata"); free(xml); @@ -1398,7 +1399,7 @@ static int testDomainCoreDump(virDomainPtr domain, "cannot save domain core"); return (-1); } - if (write(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) != sizeof(TEST_SAVE_MAGIC)) { + if (safewrite(fd, TEST_SAVE_MAGIC, sizeof(TEST_SAVE_MAGIC)) < 0) {
keep the != sizeof(TEST_SAVE_MAGIC)
testError(domain->conn, domain, NULL, VIR_ERR_INTERNAL_ERROR, "cannot write header"); close(fd); diff --git a/src/util.c b/src/util.c index f082984..c813a4c 100644 --- a/src/util.c +++ b/src/util.c @@ -1,7 +1,7 @@ /* * utils.c: common, generic utility functions * - * Copyright (C) 2006, 2007 Red Hat, Inc. + * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * Copyright (C) 2006, 2007 Binary Karma * Copyright (C) 2006 Shuveb Hussain @@ -295,26 +295,6 @@ int saferead(int fd, void *buf, size_t count) return nread; }
-/* Like write(), but restarts after EINTR */ -ssize_t safewrite(int fd, const void *buf, size_t count) -{ - size_t nwritten = 0; - while (count > 0) { - int r = write(fd, buf, count); - - if (r < 0 && errno == EINTR) - continue; - if (r < 0) - return r; - if (r == 0) - return nwritten; - buf = (unsigned char *)buf + r; - count -= r; - nwritten += r; - } - return nwritten; -} -
int __virFileReadAll(const char *path, int maxlen, diff --git a/src/util.h b/src/util.h index da1f5b0..218ab1b 100644 --- a/src/util.h +++ b/src/util.h @@ -31,7 +31,26 @@ int virExecNonBlock(virConnectPtr conn, char **argv, int *retpid, int infd, int int virRun(virConnectPtr conn, char **argv, int *status);
int saferead(int fd, void *buf, size_t count); -ssize_t safewrite(int fd, const void *buf, size_t count); + +/* Like write(), but restarts after EINTR */ +static inline ssize_t safewrite(int fd, const void *buf, size_t count) +{ + size_t nwritten = 0; + while (count > 0) { + int r = write(fd, buf, count); + + if (r < 0 && errno == EINTR) + continue; + if (r < 0) + return r; + if (r == 0) + return nwritten; + buf = (unsigned char *)buf + r; + count -= r; + nwritten += r; + } + return nwritten; +}
int __virFileReadAll(const char *path, int maxlen, diff --git a/src/virsh.c b/src/virsh.c index 730f251..b67e63c 100644 --- a/src/virsh.c +++ b/src/virsh.c @@ -4582,7 +4582,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, va_list snprintf(msg_buf + strlen(msg_buf), sizeof(msg_buf) - strlen(msg_buf), "\n");
/* write log */ - if (write(ctl->log_fd, msg_buf, strlen(msg_buf)) == -1) { + if (safewrite(ctl->log_fd, msg_buf, strlen(msg_buf)) < 0) {
!= strlen(msg_buf)
vshCloseLogFile(ctl); vshError(ctl, FALSE, "%s", _("failed to write the log file")); } -- 1.5.4.2.134.g82883
-- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-jim