On Wed, Feb 04, 2009 at 01:07:49PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Mon, Feb 02, 2009 at 06:08:17PM +0100, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com> * src/iptables.c: Include "virterror_internal.h". Use virStrerror, not strerror. * src/virterror.c (virStrerror): Remove "static". * src/virterror_internal.h (virStrerror): Declare it. ... -static const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen) +const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen) ... +const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen);
This seems to be missing ta change to libvirt_private.sym to actually export the new function to individual drivers.
Good catch. I didn't notice because currently it isn't needed.
I've fixed that and addressed all of your other comments. Here's an incremental diff that also includes additional changes, e.g., - enable the syntax-check for strerror - change more "out of error" reports to use virReportOOMError - use QEMUD_ERROR consistently
diff --git a/Makefile.nonreentrant b/Makefile.nonreentrant index 13fa59d..b567f31 100644 --- a/Makefile.nonreentrant +++ b/Makefile.nonreentrant @@ -79,7 +79,7 @@ NON_REENTRANT += setstate NON_REENTRANT += sgetspent NON_REENTRANT += srand48 NON_REENTRANT += srandom -# NON_REENTRANT += strerror +NON_REENTRANT += strerror NON_REENTRANT += strtok NON_REENTRANT += tmpnam NON_REENTRANT += ttyname diff --git a/qemud/Makefile.am b/qemud/Makefile.am index 372b931..a0c161a 100644 --- a/qemud/Makefile.am +++ b/qemud/Makefile.am @@ -107,7 +107,6 @@ libvirtd_LDADD = \ if ! WITH_DRIVER_MODULES if WITH_QEMU libvirtd_LDADD += ../src/libvirt_driver_qemu.la -libvirtd_LDADD += ../src/libvirt_util.la endif
if WITH_LXC
Someting seems not right here - we shouldn't need this change, since this shouldn't be added in the first place.
diff --git a/src/network_driver.c b/src/network_driver.c index 8d7340e..4138939 100644 --- a/src/network_driver.c +++ b/src/network_driver.c @@ -904,16 +904,17 @@ static int networkStartNetworkDaemon(virConnectPtr conn, err_delbr2: networkRemoveIptablesRules(driver, network);
- err_delbr1:; - char ebuf[1024]; + err_delbr1: if (network->def->ipAddress && (err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) { + char ebuf[1024]; networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' : %s\n"), network->def->bridge, virStrerror(err, ebuf, sizeof ebuf)); }
This also appears to be changing code which doesn't exist. [snip more of the same] Have you got the actual patch against current CVS, since this doesn't appear to be it ? Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|