On Tue, Jun 19, 2012 at 07:52:43PM +0200, Benjamin Cama wrote:
> Hi again,
>
> I forgot to port my patch to the latest git master… Here is a try, not
> tested, not even compiled; but it's less intrusive regarding the API
> thanks to the new “flags” argument.
>
> ---
> src/network/bridge_driver.c | 19 +++++++++++++++++--
> src/uml/uml_conf.c | 2 +-
> src/util/virnetdevtap.c | 24 ++++++++++++++----------
> src/util/virnetdevtap.h | 2 ++
> 4 files changed, 34 insertions(+), 13 deletions(-)
>
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index 79d3010..8f45bd7 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"
> @@ -1744,6 +1745,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 +1767,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 +1833,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"),
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.
> 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,
>
Looks reasonable apart from the cleanup bug.
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list