From: Benjamin Cama <benjamin.cama(a)telecom-bretagne.eu>
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.
---
This is my attempt at resolving the merge conflict, but I'm not
confident enough with the original patch, nor with my test
environment, to see if I actually solved anything (not to mention
the commit message used by 'git am' is horrible). Is it worth
using this version?
src/network/bridge_driver.c | 22 ++++++++++++++++++++--
src/uml/uml_conf.c | 3 ++-
src/util/virnetdevtap.c | 24 ++++++++++++++----------
src/util/virnetdevtap.h | 2 ++
4 files changed, 38 insertions(+), 13 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index fce1739..33aba74 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -65,6 +65,7 @@
#include "virnetdevtap.h"
#include "virnetdevvportprofile.h"
#include "virdbus.h"
+#include "virfile.h"
#define NETWORK_PID_DIR LOCALSTATEDIR "/run/libvirt/network"
#define NETWORK_STATE_DIR LOCALSTATEDIR "/lib/libvirt/network"
@@ -993,6 +994,7 @@ networkRadvdConfContents(virNetworkObjPtr network, char **configstr)
" AdvSendAdvert on;\n"
" AdvManagedFlag off;\n"
" AdvOtherConfigFlag off;\n"
+ " IgnoreIfMissing on;\n"
"\n",
network->def->bridge);
@@ -2061,6 +2063,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)
@@ -2082,10 +2085,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, NULL,
- VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE) <
0) {
+ NULL, &tapfd, NULL, 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;
}
@@ -2149,6 +2155,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) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot set bandwidth limits on %s"),
@@ -2187,6 +2203,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 a317bcc..e8b8779 100644
--- a/src/uml/uml_conf.c
+++ b/src/uml/uml_conf.c
@@ -142,7 +142,8 @@ umlConnectTapDevice(virConnectPtr conn,
vm->uuid, NULL,
virDomainNetGetActualVirtPortProfile(net),
virDomainNetGetActualVlan(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 9f2dca1..0eadd6c 100644
--- a/src/util/virnetdevtap.c
+++ b/src/util/virnetdevtap.c
@@ -112,18 +112,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 -1 on failure.
*/
int virNetDevTapCreate(char **ifname,
int *tapfd,
- unsigned int flags ATTRIBUTE_UNUSED)
+ unsigned int flags)
{
int fd;
struct ifreq ifr;
@@ -160,7 +162,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"),
@@ -261,14 +263,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 fa6a647..980db61 100644
--- a/src/util/virnetdevtap.h
+++ b/src/util/virnetdevtap.h
@@ -43,6 +43,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,
--
1.7.11.4