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

Re: [libvirt] [PATCH v2 1/2] util: Introduce flags field for macvtap creation



On Wed, Aug 27, 2014 at 10:34:13AM -0400, Matthew Rosato wrote:
Currently, there is one flag passed in during macvtap creation
(withTap) -- Let's convert this field to an unsigned int flag
field for future expansion.

Signed-off-by: Matthew Rosato <mjrosato linux vnet ibm com>
---
src/lxc/lxc_process.c       |    4 ++--
src/qemu/qemu_command.c     |    6 ++++--
src/util/virnetdevmacvlan.c |   28 +++++++++++++++++-----------
src/util/virnetdevmacvlan.h |   14 ++++++++++----
4 files changed, 33 insertions(+), 19 deletions(-)

diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 3353dc1..ed30c37 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -331,12 +331,12 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
            net->ifname, &net->mac,
            virDomainNetGetActualDirectDev(net),
            virDomainNetGetActualDirectMode(net),
-            false, false, def->uuid,
+            false, def->uuid,
            virDomainNetGetActualVirtPortProfile(net),
            &res_ifname,
            VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
            cfg->stateDir,
-            virDomainNetGetActualBandwidth(net)) < 0)
+            virDomainNetGetActualBandwidth(net), 0) < 0)
        goto cleanup;

    ret = res_ifname;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9241f57..1f71fa3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -177,6 +177,7 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
    char *res_ifname = NULL;
    int vnet_hdr = 0;
    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
+    unsigned int macvlan_create_flags = VIR_NETDEV_MACVLAN_CREATE_WITH_TAP;

    if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_VNET_HDR) &&
        net->model && STREQ(net->model, "virtio"))
@@ -186,11 +187,12 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
        net->ifname, &net->mac,
        virDomainNetGetActualDirectDev(net),
        virDomainNetGetActualDirectMode(net),
-        true, vnet_hdr, def->uuid,
+        vnet_hdr, def->uuid,
        virDomainNetGetActualVirtPortProfile(net),
        &res_ifname,
        vmop, cfg->stateDir,
-        virDomainNetGetActualBandwidth(net));
+        virDomainNetGetActualBandwidth(net),
+        macvlan_create_flags);
    if (rc >= 0) {
        virDomainAuditNetDevice(def, net, res_ifname, true);
        VIR_FREE(net->ifname);
diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
index 054194b..9ddeca4 100644
--- a/src/util/virnetdevmacvlan.c
+++ b/src/util/virnetdevmacvlan.c
@@ -790,6 +790,7 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
 * @macaddress: The MAC address for the macvtap device
 * @linkdev: The interface name of the NIC to connect to the external bridge
 * @mode: int describing the mode for 'bridge', 'vepa', 'private' or 'passthru'.
+ * @flags: OR of virNetDevMacVLanCreateFlags.

I took the liberty of moving this as a last parameter as well.

 * @vnet_hdr: 1 to enable IFF_VNET_HDR, 0 to disable it
 * @vmuuid: The UUID of the VM the macvtap belongs to
 * @virtPortProfile: pointer to object holding the virtual port profile data
@@ -797,25 +798,29 @@ virNetDevMacVLanVPortProfileRegisterCallback(const char *ifname,
 *     interface will be stored into if everything succeeded. It is up
 *     to the caller to free the string.
 *
- * Returns file descriptor of the tap device in case of success with @withTap,
- * otherwise returns 0; returns -1 on error.
+ * Returns file descriptor of the tap device in case of success with
+ * @flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP, otherwise returns 0; returns
+ * -1 on error.
 */
int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
                                           const virMacAddr *macaddress,
                                           const char *linkdev,
                                           virNetDevMacVLanMode mode,
-                                           bool withTap,
                                           int vnet_hdr,
                                           const unsigned char *vmuuid,
                                           virNetDevVPortProfilePtr virtPortProfile,
                                           char **res_ifname,
                                           virNetDevVPortProfileOp vmOp,
                                           char *stateDir,
-                                           virNetDevBandwidthPtr bandwidth)
+                                           virNetDevBandwidthPtr bandwidth,
+                                           unsigned int flags)
{
-    const char *type = withTap ? "macvtap" : "macvlan";
-    const char *prefix = withTap ? MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX;
-    const char *pattern = withTap ? MACVTAP_NAME_PATTERN : MACVLAN_NAME_PATTERN;
+    const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+        "macvtap" : "macvlan";
+    const char *prefix = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+        MACVTAP_NAME_PREFIX : MACVLAN_NAME_PREFIX;
+    const char *pattern = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
+        MACVTAP_NAME_PATTERN : MACVLAN_NAME_PATTERN;
    int c, rc;
    char ifname[IFNAMSIZ];
    int retries, do_retry = 0;
@@ -903,7 +908,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
        goto disassociate_exit;
    }

-    if (withTap) {
+    if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) {
        if ((rc = virNetDevMacVLanTapOpen(cr_ifname, 10)) < 0)
            goto disassociate_exit;

@@ -922,7 +927,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *tgifname,
    }

    if (virNetDevBandwidthSet(cr_ifname, bandwidth, false) < 0) {
-        if (withTap)
+        if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP)
            VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
        else
            rc = -1;
@@ -1066,15 +1071,16 @@ int virNetDevMacVLanCreateWithVPortProfile(const char *ifname ATTRIBUTE_UNUSED,
                                           const virMacAddr *macaddress ATTRIBUTE_UNUSED,
                                           const char *linkdev ATTRIBUTE_UNUSED,
                                           virNetDevMacVLanMode mode ATTRIBUTE_UNUSED,
-                                           bool withTap ATTRIBUTE_UNUSED,
                                           int vnet_hdr ATTRIBUTE_UNUSED,
                                           const unsigned char *vmuuid ATTRIBUTE_UNUSED,
                                           virNetDevVPortProfilePtr virtPortProfile ATTRIBUTE_UNUSED,
                                           char **res_ifname ATTRIBUTE_UNUSED,
                                           virNetDevVPortProfileOp vmop ATTRIBUTE_UNUSED,
                                           char *stateDir ATTRIBUTE_UNUSED,
-                                           virNetDevBandwidthPtr bandwidth ATTRIBUTE_UNUSED)
+                                           virNetDevBandwidthPtr bandwidth ATTRIBUTE_UNUSED,
+                                           unsigned int flags)
{
+    virCheckFlags(0, NULL);

The flag check could be fixed not to report in such cases, but it's
easier just to check the flags for now.  But it should be:

virCheckFlags(0, -1);

    virReportSystemError(ENOSYS, "%s",
                         _("Cannot create macvlan devices on this platform"));
    return -1;
diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h
index 9b15a31..e92da4a 100644
--- a/src/util/virnetdevmacvlan.h
+++ b/src/util/virnetdevmacvlan.h
@@ -40,6 +40,12 @@ typedef enum {
} virNetDevMacVLanMode;
VIR_ENUM_DECL(virNetDevMacVLanMode)

+typedef enum {
+   VIR_NETDEV_MACVLAN_CREATE_NONE = 0,
+   /* Create with a tap device */
+   VIR_NETDEV_MACVLAN_CREATE_WITH_TAP       = 1 << 0,

You aligned only one value.  Unless you object to it, I'll align them
together without too many whitespaces.

Other than that, it looks fine, ACK after 1.2.8, I'll push it with the
changes mentioned after the release if you're OK with that.

Martin

Attachment: signature.asc
Description: Digital signature


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