"Daniel P. Berrange" <berrange@redhat.com> wrote:

The strerror() method is not guarenteed to be re-entrant, which is ...

@@ -759,8 +762,8 @@ static int networkStartNetworkDaemon(vir

if (network->def->forwardType != VIR_NETWORK_FORWARD_NONE && !networkEnableIpForwarding()) { - networkReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - _("failed to enable IP forwarding : %s"), strerror(err)); + virReportSystemError(conn, errno, "%s", + _("failed to enable IP forwarding")); goto err_delbr2; }

Looking at this chunk and the s/err/errno/ change (that's a bug fix), I spotted another small bug: Any write error in networkEnableIpForwarding was ignored, I added a check for failed close, too, just in case. Jim

From b4abd36014b56cdb33a91ab336eff6450a5d3c8a Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Mon, 19 Jan 2009 10:11:15 +0100 Subject: [PATCH] don't ignore write failure

* src/network_driver.c (networkEnableIpForwarding): This function would always return 1 (success). Now it returns 0 upon write or close failure, being careful to preserve any errno from a write failure. Add comments. --- src/network_driver.c | 20 ++++++++++++++------ 1 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/network_driver.c b/src/network_driver.c index 695acfb..51cfbbd 100644 --- a/src/network_driver.c +++ b/src/network_driver.c @@ -1,7 +1,7 @@ /* * driver.c: core driver methods for managing qemu guests * - * Copyright (C) 2006, 2007, 2008 Red Hat, Inc. + * Copyright (C) 2006-2009 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -774,20 +774,29 @@ networkRemoveIptablesRules(struct network_driver *driver, iptablesSaveRules(driver->iptables); } +/* Enable IP Forwarding. + Return 0 for success, nonzero for failure. + Be careful to preserve any errno value upon failure. */ static int networkEnableIpForwarding(void) { #define PROC_IP_FORWARD "/proc/sys/net/ipv4/ip_forward" - int fd, ret; + int fd; if ((fd = open(PROC_IP_FORWARD, O_WRONLY|O_TRUNC)) == -1) return 0; - if (safewrite(fd, "1\n", 2) < 0) - ret = 0; + if (safewrite(fd, "1\n", 2) < 0) { + int saved_errno = errno; + close (fd); + errno = saved_errno; + return 0; + } - close (fd); + /* Use errno from failed close only if there was no write error. */ + if (close (fd) != 0) + return 0; return 1; @@ -1429,4 +1438,3 @@ int networkRegister(void) { virRegisterStateDriver(&networkStateDriver); return 0; } - -- 1.6.1.258.g7ff14