[libvirt] [PATCH 4/9] iptables.c: Use virStrerror, not strerror.

[Daniel P. Berrange]

On Wed, Feb 04, 2009 at 01:07:49PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> > On Mon, Feb 02, 2009 at 06:08:17PM +0100, Jim Meyering wrote:
> >> From: Jim Meyering <meyering at 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 :|