Le mercredi 20 juin 2012 à 11:05 +0100, Daniel P. Berrange a écrit :
I think you also need to have a VIR_FOFRCE_CLOSE(tapfd) in the
very last of the cleanup jump labels. Since between the time
we open the tapfd & close it, we might have done a 'goto err1'
or similar.
Yes, I forgot that. New patch following.
Looks reasonable apart from the cleanup bug.
Thanks. BTW, if this patch is commited, I'm already in AUTHORS under
another email address, which is OK.
---
src/network/bridge_driver.c | 22 ++++++++++++++++++++--
src/uml/uml_conf.c | 2 +-
src/util/virnetdevtap.c | 24 ++++++++++++++----------
src/util/virnetdevtap.h | 2 ++
4 files changed, 37 insertions(+), 13 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 79d3010..e9f3c90 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -62,6 +62,7 @@
#include "virnetdev.h"
#include "virnetdevbridge.h"
#include "virnetdevtap.h"
+#include "virfile.h"
#define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
#define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -840,6 +841,7 @@ networkStartRadvd(virNetworkObjPtr network)
" AdvSendAdvert on;\n"
" AdvManagedFlag off;\n"
" AdvOtherConfigFlag off;\n"
+ " IgnoreIfMissing on;\n"
"\n",
network->def->bridge);
for (ii = 0;
@@ -1744,6 +1746,7 @@ networkStartNetworkVirtual(struct network_driver *driver,
virErrorPtr save_err = NULL;
virNetworkIpDefPtr ipdef;
char *macTapIfName = NULL;
+ int tapfd = -1;
/* Check to see if any network IP collides with an existing route */
if (networkCheckRouteCollision(network) < 0)
@@ -1765,10 +1768,13 @@ networkStartNetworkVirtual(struct network_driver *driver,
virReportOOMError();
goto err0;
}
+ /* Keep tun fd open and interface up to allow for IPv6 DAD to happen */
if (virNetDevTapCreateInBridgePort(network->def->bridge,
&macTapIfName, network->def->mac,
- NULL, NULL, NULL,
- VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE) <
0) {
+ NULL, &tapfd, NULL,
+ VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE
+ |VIR_NETDEV_TAP_CREATE_IFUP
+ |VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
VIR_FREE(macTapIfName);
goto err0;
}
@@ -1828,6 +1834,16 @@ networkStartNetworkVirtual(struct network_driver *driver,
if (v6present && networkStartRadvd(network) < 0)
goto err4;
+ /* DAD has happened (dnsmasq waits for it), dnsmasq is now bound to the
+ * bridge's IPv6 address, and radvd to the interface, so we can now set the
+ * dummy tun down.
+ */
+ if (tapfd >= 0) {
+ if (virNetDevSetOnline(macTapIfName, false) < 0)
+ goto err4;
+ VIR_FORCE_CLOSE(tapfd);
+ }
+
if (virNetDevBandwidthSet(network->def->bridge, network->def->bandwidth)
< 0) {
networkReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot set bandwidth limits on %s"),
@@ -1866,6 +1882,8 @@ networkStartNetworkVirtual(struct network_driver *driver,
save_err = virSaveLastError();
if (macTapIfName) {
+ if (tapfd >= 0)
+ VIR_FORCE_CLOSE(tapfd);
ignore_value(virNetDevTapDelete(macTapIfName));
VIR_FREE(macTapIfName);
}
diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c
index 79b249d..c9b5044 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -141,7 +141,7 @@ umlConnectTapDevice(virConnectPtr conn,
if (virNetDevTapCreateInBridgePort(bridge, &net->ifname, net->mac,
vm->uuid, NULL,
virDomainNetGetActualVirtPortProfile(net),
- VIR_NETDEV_TAP_CREATE_IFUP) < 0) {
+
VIR_NETDEV_TAP_CREATE_IFUP|VIR_NETDEV_TAP_CREATE_PERSIST) < 0) {
if (template_ifname)
VIR_FREE(net->ifname);
goto error;
diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c
index 5d21164..45ff20f 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -118,18 +118,20 @@ virNetDevProbeVnetHdr(int tapfd)
*
* VIR_NETDEV_TAP_CREATE_VNET_HDR
* - Enable IFF_VNET_HDR on the tap device
+ * VIR_NETDEV_TAP_CREATE_PERSIST
+ * - The device will persist after the file descriptor is closed
*
* 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 virNetDevTapDelete to
- * remove a persistent TAP devices when it is no longer needed.
+ * If the @tapfd parameter is supplied, the open tap device file descriptor
+ * will be returned, otherwise the TAP device will be closed. The caller must
+ * use virNetDevTapDelete to remove a persistent TAP device 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 *tapfd,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
int fd;
struct ifreq ifr;
@@ -166,7 +168,7 @@ int virNetDevTapCreate(char **ifname,
goto cleanup;
}
- if (!tapfd &&
+ if ((flags & VIR_NETDEV_TAP_CREATE_PERSIST) &&
(errno = ioctl(fd, TUNSETPERSIST, 1))) {
virReportSystemError(errno,
_("Unable to set tap device %s to persistent"),
@@ -267,14 +269,16 @@ int virNetDevTapDelete(const char *ifname ATTRIBUTE_UNUSED)
* - 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
+ * VIR_NETDEV_TAP_CREATE_PERSIST
+ * - The device will persist after the file descriptor is closed
*
* 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 virNetDevTapDelete to remove
- * a persistent TAP devices when it is no longer needed.
+ * If the @tapfd parameter is supplied, the open tap device file descriptor
+ * will be returned, otherwise the TAP device will be closed. The caller must
+ * use virNetDevTapDelete to remove a persistent TAP device when it is no
+ * longer needed.
*
* Returns 0 in case of success or -1 on failure
*/
diff --git a/src/util/virnetdevtap.h b/src/util/virnetdevtap.h
index d9a3593..f1355ba 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -42,6 +42,8 @@ typedef enum {
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,
+ /* The device will persist after the file descriptor is closed */
+ VIR_NETDEV_TAP_CREATE_PERSIST = 1 << 3,
} virNetDevTapCreateFlags;
int virNetDevTapCreateInBridgePort(const char *brname,
--
Benjamin Cama <benjamin.cama(a)telecom-bretagne.eu>