[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] util: combine three bools in virNetDevTapCreateInBridgePort into one flags





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.

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]