[libvirt] [PATCH] util: combine three bools in virNetDevTapCreateInBridgePort into one flags
Ansis Atteka
aatteka at nicira.com
Fri Mar 2 05:46:36 UTC 2012
On Thu, Mar 1, 2012 at 12:48 PM, Laine Stump <laine at 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 at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120301/308c0923/attachment-0001.htm>
More information about the libvir-list
mailing list