[libvirt] [PATCH 1/2] BSD: implement virNetDevBridgeCreate() and virNetDevBridgeDelete()

[John Ferlan]

On 06/19/2013 12:47 PM, Roman Bogorodskiy wrote:
> Implementation uses SIOCIFCREATE2 and SIOCIFDESTROY ioctls.
> Also, drop static virNetDevSetupControl() as we have
> public one avialable now.


Dropping the static/local virNetDevSetupControlFull() and
virNetDevSetupControl() seems to have triggered another side effect with
my daily Coverity build which uses "./autogen.sh --system
lv_cv_static_analysis=yes" prior to building.

I'm not sure how else to trigger it or all

The definition of the function is as follows:

int virNetDevSetupControl(const char *ifname,
                          virIfreq *ifr)
    ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);


However, there's two callers in virnetdevbridge.c that call it with
"NULL, NULL":


#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRADDBR)
int virNetDevBridgeCreate(const char *brname)
...

    if ((fd = virNetDevSetupControl(NULL, NULL)) < 0)
        return -1;
...


and
#if defined(HAVE_STRUCT_IFREQ) && defined(SIOCBRDELBR)
int virNetDevBridgeDelete(const char *brname)
...
    if ((fd = virNetDevSetupControl(NULL, NULL)) < 0)
        return -1;
...


That's because internal.h has:

#  ifndef ATTRIBUTE_NONNULL
#   if __GNUC_PREREQ (3, 3)
#    if STATIC_ANALYSIS
#     define ATTRIBUTE_NONNULL(m) __attribute__((__nonnull__(m)))
#    else
#     define ATTRIBUTE_NONNULL(m) __attribute__(())
#    endif
#   else
#    define ATTRIBUTE_NONNULL(m)
#   endif
#  endif


John

> ---
>  src/util/virnetdevbridge.c | 91 ++++++++++++++++++++++++----------------------
>  1 file changed, 48 insertions(+), 43 deletions(-)
> 
> diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c
> index 130829c..df606d2 100644
> --- a/src/util/virnetdevbridge.c
> +++ b/src/util/virnetdevbridge.c
> @@ -23,6 +23,7 @@
>  #include <config.h>
>  
>  #include "virnetdevbridge.h"
> +#include "virnetdev.h"
>  #include "virerror.h"
>  #include "virutil.h"
>  #include "virfile.h"
> @@ -48,49 +49,6 @@
>  
>  
>  #if defined(HAVE_STRUCT_IFREQ) && defined(__linux__)
> -static int virNetDevSetupControlFull(const char *ifname,
> -                                     struct ifreq *ifr,
> -                                     int domain,
> -                                     int type)
> -{
> -    int fd;
> -
> -    if (ifname && ifr) {
> -        memset(ifr, 0, sizeof(*ifr));
> -
> -        if (virStrcpyStatic(ifr->ifr_name, ifname) == NULL) {
> -            virReportSystemError(ERANGE,
> -                                 _("Network interface name '%s' is too long"),
> -                                 ifname);
> -            return -1;
> -        }
> -    }
> -
> -    if ((fd = socket(domain, type, 0)) < 0) {
> -        virReportSystemError(errno, "%s",
> -                             _("Cannot open network interface control socket"));
> -        return -1;
> -    }
> -
> -    if (virSetInherit(fd, false) < 0) {
> -        virReportSystemError(errno, "%s",
> -                             _("Cannot set close-on-exec flag for socket"));
> -        VIR_FORCE_CLOSE(fd);
> -        return -1;
> -    }
> -
> -    return fd;
> -}
> -
> -
> -static int virNetDevSetupControl(const char *ifname,
> -                                 struct ifreq *ifr)
> -{
> -    return virNetDevSetupControlFull(ifname, ifr, AF_PACKET, SOCK_DGRAM);
> -}
> -#endif
> -
> -#if defined(HAVE_STRUCT_IFREQ) && defined(__linux__)
>  # define SYSFS_NET_DIR "/sys/class/net"
>  /*
>   * Bridge parameters can be set via sysfs on newish kernels,
> @@ -233,6 +191,31 @@ cleanup:
>      VIR_FORCE_CLOSE(fd);
>      return ret;
>  }
> +#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCIFCREATE2)
> +int virNetDevBridgeCreate(const char *brname)
> +{
> +    int s;
> +    struct ifreq ifr;
> +    int ret = - 1;
> +
> +    if ((s = virNetDevSetupControl("bridge", &ifr)) < 0)
> +        return -1;
> +
> +    if (ioctl(s, SIOCIFCREATE2, &ifr) < 0) {
> +        virReportSystemError(errno, "%s",
> +                             _("Unable to create bridge device"));
> +        goto cleanup;
> +    }
> +
> +    if (virNetDevSetName(ifr.ifr_name, brname) == -1) {
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> +cleanup:
> +    VIR_FORCE_CLOSE(s);
> +    return ret;
> +}
>  #else
>  int virNetDevBridgeCreate(const char *brname)
>  {
> @@ -271,6 +254,28 @@ cleanup:
>      VIR_FORCE_CLOSE(fd);
>      return ret;
>  }
> +#elif defined(HAVE_STRUCT_IFREQ) && defined(SIOCIFDESTROY)
> +int virNetDevBridgeDelete(const char *brname)
> +{
> +    int s;
> +    struct ifreq ifr;
> +    int ret = -1;
> +
> +    if ((s = virNetDevSetupControl(brname, &ifr)) < 0)
> +        return -1;
> +
> +    if (ioctl(s, SIOCIFDESTROY, &ifr) < 0) {
> +        virReportSystemError(errno,
> +                             _("Unable to remove bridge %s"),
> +                             brname);
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> +cleanup:
> +    VIR_FORCE_CLOSE(s);
> +    return ret;
> +}
>  #else
>  int virNetDevBridgeDelete(const char *brname ATTRIBUTE_UNUSED)
>  {
>