On Thu, Mar 1, 2012 at 12:48 PM, Laine Stump
<laine@laine.org> wrote:
With an additional new bool added to determine whether or not to
discourage the use of the supplied MAC address by the bridge itself,
virNetDevTapCreateInBridgePort had three booleans (well, 2 bools and
an int used as a bool) in the arg list, which made it increasingly
difficult to follow what was going on. This patch combines those three
into a single flags arg, which not only shortens the arg list, but
makes it more self-documenting.
---
Does this make more sense as a PATCH 2/1 to be associated with the
first patch in this thread:
http://www.redhat.com/archives/libvir-list/2012-February/msg00760.html
or should I squash them both together? (I'm leaning towards two
separate patches, but could be convinced either way)
src/network/bridge_driver.c | 3 +-
src/qemu/qemu_command.c | 13 ++++++-----
src/uml/uml_conf.c | 8 +++---
src/util/virnetdevtap.c | 46 +++++++++++++++++++++++++-----------------
src/util/virnetdevtap.h | 20 +++++++++++++-----
5 files changed, 54 insertions(+), 36 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 3e1e031..cf75d26 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1766,7 +1766,8 @@ networkStartNetworkVirtual(struct network_driver *driver,
}
if (virNetDevTapCreateInBridgePort(network->def->bridge,
&macTapIfName, network->def->mac,
- false, 0, false, NULL, NULL) < 0) {
+ NULL, NULL,
+ VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE) < 0) {
VIR_FREE(macTapIfName);
goto err0;
}
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 7e121c7..acfd38c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -178,7 +178,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
char *brname = NULL;
int err;
int tapfd = -1;
- int vnet_hdr = 0;
+ unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP;
bool template_ifname = false;
int actualType = virDomainNetGetActualType(net);
@@ -240,12 +240,13 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
}
if (qemuCapsGet(qemuCaps, QEMU_CAPS_VNET_HDR) &&
- net->model && STREQ(net->model, "virtio"))
- vnet_hdr = 1;
+ net->model && STREQ(net->model, "virtio")) {
+ tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR;
+ }
- err = virNetDevTapCreateInBridgePort(brname, &net->ifname, net->mac, true,
- vnet_hdr, true, &tapfd,
- virDomainNetGetActualVirtPortProfile(net));
+ err = virNetDevTapCreateInBridgePort(brname, &net->ifname, net->mac, &tapfd,
+ virDomainNetGetActualVirtPortProfile(net),
+ tap_create_flags);
virDomainAuditNetDevice(def, net, "/dev/net/tun", tapfd >= 0);
if (err < 0) {
if (template_ifname)
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index c7b29a0..89fdd9f 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -1,7 +1,7 @@
/*
* uml_conf.c: UML driver configuration
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2012 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -138,9 +138,9 @@ umlConnectTapDevice(virConnectPtr conn,
template_ifname = true;
}
- if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac, true,
- 0, true, NULL,
- virDomainNetGetActualVirtPortProfile(net)) < 0) {
+ if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac, NULL,
+ virDomainNetGetActualVirtPortProfile(net),
+ VIR_NETDEV_TAP_CREATE_IFUP) < 0) {
if (template_ifname)
VIR_FREE(net->ifname);
goto error;
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 868ba57..fb0a8d2 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -107,22 +107,25 @@ virNetDevProbeVnetHdr(int tapfd)
#ifdef TUNSETIFF
/**
- * brCreateTap:
+ * virNetDevTapCreate:
* @ifname: the interface name
- * @vnet_hr: whether to try enabling IFF_VNET_HDR
* @tapfd: file descriptor return value for the new tap device
+ * @flags: OR of virNetDevTapCreateFlags. Only one flag is recognized:
+ *
+ * VIR_NETDEV_TAP_CREATE_VNET_HDR
+ * - Enable IFF_VNET_HDR on the 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.
+ * persistent and closed. The caller must use virNetDevTapDelete 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.
*/
int virNetDevTapCreate(char **ifname,
- int vnet_hdr ATTRIBUTE_UNUSED,
- int *tapfd)
+ int *tapfd,
+ unsigned int flags ATTRIBUTE_UNUSED)
{
int fd;
struct ifreq ifr;
@@ -139,7 +142,8 @@ int virNetDevTapCreate(char **ifname,
ifr.ifr_flags = IFF_TAP|IFF_NO_PI;
# ifdef IFF_VNET_HDR
- if (vnet_hdr && virNetDevProbeVnetHdr(fd))
+ if ((flags & VIR_NETDEV_TAP_CREATE_VNET_HDR) &&
+ virNetDevProbeVnetHdr(fd))
ifr.ifr_flags |= IFF_VNET_HDR;
# endif
@@ -228,8 +232,8 @@ cleanup:
}
#else /* ! TUNSETIFF */
int virNetDevTapCreate(char **ifname ATTRIBUTE_UNUSED,
- int vnet_hdr ATTRIBUTE_UNUSED,
- int *tapfd ATTRIBUTE_UNUSED)
+ int *tapfd ATTRIBUTE_UNUSED,
+ unsigned int flags ATTRIBUTE_UNUSED)
{
virReportSystemError(ENOSYS, "%s",
_("Unable to create TAP devices on this platform"));
@@ -249,17 +253,23 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)
* @brname: the bridge name
* @ifname: the interface name (or name template)
* @macaddr: desired MAC address (VIR_MAC_BUFLEN long)
- * @discourage: whether bridge should be discouraged from using macaddr
- * @vnet_hdr: whether to try enabling IFF_VNET_HDR
* @tapfd: file descriptor return value for the new tap device
* @virtPortProfile: bridge/port specific configuration
+ * @flags: OR of virNetDevTapCreateFlags:
+
+ * VIR_NETDEV_TAP_CREATE_IFUP
+ * - Bring the interface up
+ * VIR_NETDEV_TAP_CREATE_VNET_HDR
+ * - Enable IFF_VNET_HDR on the tap device
+ * VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE
+ * - Set this interface's MAC as the bridge's MAC address
*
* This function creates a new tap device on a bridge. @ifname can be either
* a fixed name or a name template with '%d' for dynamic name allocation.
* in either case the final name for the bridge will be stored in @ifname.
* 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
+ * persistent and closed. The caller must use virNetDevTapDelete to remove
* a persistent TAP devices when it is no longer needed.
*
* Returns 0 in case of success or -1 on failure
@@ -267,15 +277,13 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)
int virNetDevTapCreateInBridgePort(const char *brname,
char **ifname,
const unsigned char *macaddr,
- bool discourage,
- int vnet_hdr,
- bool up,
int *tapfd,
- virNetDevVPortProfilePtr virtPortProfile)
+ virNetDevVPortProfilePtr virtPortProfile,
+ unsigned int flags)
{
unsigned char tapmac[VIR_MAC_BUFLEN];
- if (virNetDevTapCreate(ifname, vnet_hdr, tapfd) < 0)
+ if (virNetDevTapCreate(ifname, tapfd, flags) < 0)
return -1;
/* We need to set the interface MAC before adding it
@@ -285,7 +293,7 @@ int virNetDevTapCreateInBridgePort(const char *brname,
* device before we set our static MAC.
*/
memcpy(tapmac, macaddr, VIR_MAC_BUFLEN);
- if (discourage)
+ if (!(flags & VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE))
tapmac[0] = 0xFE; /* Discourage bridge from using TAP dev MAC */
if (virNetDevSetMAC(*ifname, tapmac) < 0)
@@ -308,7 +316,7 @@ int virNetDevTapCreateInBridgePort(const char *brname,
goto error;
}
- if (virNetDevSetOnline(*ifname, up) < 0)
+ if (virNetDevSetOnline(*ifname, !!(flags & VIR_NETDEV_TAP_CREATE_IFUP)) < 0)
goto error;
return 0;
diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
index fc50e22..971b166 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -27,21 +27,29 @@
# include "virnetdevvportprofile.h"
int virNetDevTapCreate(char **ifname,
- int vnet_hdr,
- int *tapfd)
+ int *tapfd,
+ unsigned int flags)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virNetDevTapDelete(const char *ifname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
+typedef enum {
+ VIR_NETDEV_TAP_CREATE_NONE = 0,
+ /* Bring the interface up */
+ VIR_NETDEV_TAP_CREATE_IFUP = 1 << 0,
+ /* Enable IFF_VNET_HDR on the tap device */
+ VIR_NETDEV_TAP_CREATE_VNET_HDR = 1 << 1,
+ /* Set this interface's MAC as the bridge's MAC address */
+ VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE = 1 << 2,
+} virNetDevTapCreateFlags;
+
int virNetDevTapCreateInBridgePort(const char *brname,
char **ifname,
const unsigned char *macaddr,
- bool discourage,
- int vnet_hdr,
- bool up,
int *tapfd,
- virNetDevVPortProfilePtr virtPortProfile)
+ virNetDevVPortProfilePtr virtPortProfile,
+ unsigned int flags)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_RETURN_CHECK;
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Tested this patch together with 2/1 and it works fine for me. I will soon send out the 3/1 patch
that sets the vm-uuid.