Long subject line; I trimmed it: bridge: modify for use when sVirt is enabled with qemu Use 'git shortlog -30' to get a feel for typical subjects. On 10/24/2011 05:43 PM, Tyler Coumbes wrote:
This refactors the TAP creation code out of brAddTap into a new function brCreateTap to allow it to be used on its own. I have also changed ifSetInterfaceMac to brSetInterfaceMac and exported it since it is will be needed by code outside of util/bridge.c in the next patch. AUTHORS | 1 + src/libvirt_bridge.syms | 2 + src/util/bridge.c | 116 +++++++++++++++++++++++++++++++---------------- src/util/bridge.h | 9 ++++ 4 files changed, 89 insertions(+), 39 deletions(-) +++ b/src/libvirt_bridge.syms @@ -12,10 +12,12 @@ brAddTap; brDeleteTap; brDeleteBridge; brDelInetAddress; +brCreateTap;
Not sorted, but easy to fix.
-static int ifSetInterfaceMac(brControl *ctl, const char *ifname, +int brSetInterfaceMac(brControl *ctl, const char *ifname, const unsigned char *macaddr)
I touched up the indentation here to match.
{ struct ifreq ifr; @@ -478,32 +478,12 @@ brAddTap(brControl *ctl, bool up, int *tapfd) { - int fd; - struct ifreq ifr; - if (!ctl || !ctl->fd || !bridge || !ifname) return EINVAL; - if ((fd = open("/dev/net/tun", O_RDWR))< 0) - return errno; - - memset(&ifr, 0, sizeof(ifr)); - - ifr.ifr_flags = IFF_TAP|IFF_NO_PI; + errno = brCreateTap(ctl, ifname, vnet_hdr, tapfd);
Generally, we prefer returning -1 on failure, rather than a positive errno value; but this is pre-existing.
-# ifdef IFF_VNET_HDR - if (vnet_hdr&& brProbeVnetHdr(fd)) - ifr.ifr_flags |= IFF_VNET_HDR; -# else - (void) vnet_hdr; -# endif - - if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) { - errno = EINVAL; - goto error; - } - - if (ioctl(fd, TUNSETIFF,&ifr)< 0) + if(*tapfd< 0 || errno)
space after if.
}
+/** + * brCreateTap: + * @ctl: bridge control pointer + * @ifname: the interface name + * @vnet_hr: whether to try enabling IFF_VNET_HDR + * @tapfd: file descriptor return value for the new tap device + * + * Creates a tap interface. + * If the @tapfd parameter is supplied, the open tap device file + * descriptor will be returned, otherwise the TAP device will be made + * persistent and closed. The caller must use brDeleteTap to remove + * a persistent TAP devices when it is no longer needed. + * + * Returns 0 in case of success or an errno code in case of failure.
So your choice of convention isn't typical, but fits the existing code.
+ */ + +int brCreateTap (brControl *ctl ATTRIBUTE_UNUSED,
No space after function names.
+ char **ifname, + int vnet_hdr, + int *tapfd) +{ + + int fd;
Extra blank line.
+ struct ifreq ifr; + + if (!ifname) + return EINVAL; + + if ((fd = open("/dev/net/tun", O_RDWR))< 0) + return errno;
Inconsistent indentation.
+ + memset(&ifr, 0, sizeof(ifr)); + + ifr.ifr_flags = IFF_TAP|IFF_NO_PI; + +# ifdef IFF_VNET_HDR + if (vnet_hdr&& brProbeVnetHdr(fd)) + ifr.ifr_flags |= IFF_VNET_HDR; +# else + (void) vnet_hdr;
I don't like the cast to void; marking the parameter ATTRIBUTE_UNUSED is sufficient.
But those are minor. So: ACK. I squashed this in and pushed. diff --git i/src/libvirt_bridge.syms w/src/libvirt_bridge.syms index 669830b..626f6ee 100644 --- i/src/libvirt_bridge.syms +++ w/src/libvirt_bridge.syms @@ -9,10 +9,10 @@ brAddBridge; brAddInetAddress; brAddInterface; brAddTap; -brDeleteTap; -brDeleteBridge; -brDelInetAddress; brCreateTap; +brDelInetAddress; +brDeleteBridge; +brDeleteTap; brHasBridge; brInit; brSetEnableSTP; diff --git i/src/util/bridge.c w/src/util/bridge.c index 4875f52..952f0f3 100644 --- i/src/util/bridge.c +++ w/src/util/bridge.c @@ -288,8 +288,9 @@ brDeleteInterface(brControl *ctl ATTRIBUTE_UNUSED, * * Returns 0 in case of success or an errno code in case of failure. */ -int brSetInterfaceMac(brControl *ctl, const char *ifname, - const unsigned char *macaddr) +int +brSetInterfaceMac(brControl *ctl, const char *ifname, + const unsigned char *macaddr) { struct ifreq ifr; @@ -483,7 +484,7 @@ brAddTap(brControl *ctl, errno = brCreateTap(ctl, ifname, vnet_hdr, tapfd); - if(*tapfd < 0 || errno) + if (*tapfd < 0 || errno) goto error; /* We need to set the interface MAC before adding it @@ -789,12 +790,12 @@ cleanup: * Returns 0 in case of success or an errno code in case of failure. */ -int brCreateTap (brControl *ctl ATTRIBUTE_UNUSED, - char **ifname, - int vnet_hdr, - int *tapfd) +int +brCreateTap(brControl *ctl ATTRIBUTE_UNUSED, + char **ifname, + int vnet_hdr ATTRIBUTE_UNUSED, + int *tapfd) { - int fd; struct ifreq ifr; @@ -802,7 +803,7 @@ int brCreateTap (brControl *ctl ATTRIBUTE_UNUSED, return EINVAL; if ((fd = open("/dev/net/tun", O_RDWR)) < 0) - return errno; + return errno; memset(&ifr, 0, sizeof(ifr)); @@ -811,8 +812,6 @@ int brCreateTap (brControl *ctl ATTRIBUTE_UNUSED, # ifdef IFF_VNET_HDR if (vnet_hdr && brProbeVnetHdr(fd)) ifr.ifr_flags |= IFF_VNET_HDR; -# else - (void) vnet_hdr; # endif if (virStrcpyStatic(ifr.ifr_name, *ifname) == NULL) { -- Eric Blake eblake@xxxxxxxxxx +1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list