[Qemu-devel] [PATCH] ifname=xxx for -netdev bridge

* [Qemu-devel] [PATCH] ifname=xxx for -netdev bridge
@ 2013-03-21 18:05 Alexandre Kandalintsev
  2013-03-21 18:57 ` Eric Blake
  2013-03-25  9:03 ` [Qemu-devel] [PATCH] " Stefan Hajnoczi
  0 siblings, 2 replies; 8+ messages in thread
From: Alexandre Kandalintsev @ 2013-03-21 18:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: Anthony Liguori, Stefan Hajnoczi

Hi!


Here is the patch that allows us to specify the name of tap interface
when -netdev bridge is used. It's like -netdev tap,ifname=xxx, but for
bridges.


** Motivation **

We've got zillions of VMs and would like to see meaningful names of tap
interfaces. This is really useful for for, e.g., system administrators
in case they want to run tcpdump on it.


** How it works **

Just specify a ifname= parameter as it is done if --netdev tap is used.
However, as it requires root privs, the interface renaming is
actually done by qemu-bridge-helper. --netdev tap,ifname=xxx will fail
if qemu is launched not from root.


** TODO **

1. Update docs
2. I'm afraid that net_init_tap should not run helper with
--br=DEFAULT_BRIDGE_INTERFACE . At least bridge name should be tunnable.
But this is a future work.
3. May be we should call qemu-bridge-helper for tap interface renamings
because it always has root privs?


From 079027fe3696de2e2adc8e60377b995dd9548eac Mon Sep 17 00:00:00 2001
From: Alexandre Kandalintsev <spam@messir.net>
Date: Thu, 21 Mar 2013 18:48:12 +0100
Subject: [PATCH] added support for ifname in -netdev bridge


Signed-off-by: Alexandre Kandalintsev <spam@messir.net>
---
 include/net/net.h    |  1 +
 net/tap.c            | 25 ++++++++++++++++---------
 qapi-schema.json     |  1 +
 qemu-bridge-helper.c | 12 ++++++++++--
 4 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index cb049a1..4b25eb5 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -172,6 +172,7 @@ NetClientState *net_hub_port_find(int hub_id);
 #define DEFAULT_NETWORK_DOWN_SCRIPT "/etc/qemu-ifdown"
 #define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR "/qemu-bridge-helper"
 #define DEFAULT_BRIDGE_INTERFACE "br0"
+#define DEFAULT_BRIDGE_IFNAME "tap%d"
 
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
 
diff --git a/net/tap.c b/net/tap.c
index daab350..d2e4bfc 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -424,11 +424,11 @@ static int recv_fd(int c)
     return len;
 }
 
-static int net_bridge_run_helper(const char *helper, const char *bridge)
+static int net_bridge_run_helper(const char *helper, const char *bridge, const char *ifname)
 {
     sigset_t oldmask, mask;
     int pid, status;
-    char *args[5];
+    char *args[6];
     char **parg;
     int sv[2];
 
@@ -446,6 +446,7 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
         int open_max = sysconf(_SC_OPEN_MAX), i;
         char fd_buf[6+10];
         char br_buf[6+IFNAMSIZ] = {0};
+        char ifname_buf[9+IFNAMSIZ] = {0};
         char helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + 15];
 
         for (i = 0; i < open_max; i++) {
@@ -459,6 +460,10 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
 
         snprintf(fd_buf, sizeof(fd_buf), "%s%d", "--fd=", sv[1]);
 
+        if (ifname) {
+          snprintf(ifname_buf, sizeof(ifname_buf), "%s%s", "--ifname=", ifname);
+        }
+
         if (strrchr(helper, ' ') || strrchr(helper, '\t')) {
             /* assume helper is a command */
 
@@ -466,8 +471,8 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
                 snprintf(br_buf, sizeof(br_buf), "%s%s", "--br=", bridge);
             }
 
-            snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s",
-                     helper, "--use-vnet", fd_buf, br_buf);
+            snprintf(helper_cmd, sizeof(helper_cmd), "%s %s %s %s %s",
+                     helper, "--use-vnet", fd_buf, br_buf, ifname_buf);
 
             parg = args;
             *parg++ = (char *)"sh";
@@ -486,6 +491,7 @@ static int net_bridge_run_helper(const char *helper, const char *bridge)
             *parg++ = (char *)"--use-vnet";
             *parg++ = fd_buf;
             *parg++ = br_buf;
+            *parg++ = ifname_buf;
             *parg++ = NULL;
 
             execv(helper, args);
@@ -524,7 +530,7 @@ int net_init_bridge(const NetClientOptions *opts, const char *name,
                     NetClientState *peer)
 {
     const NetdevBridgeOptions *bridge;
-    const char *helper, *br;
+    const char *helper, *br, *ifname;
 
     TAPState *s;
     int fd, vnet_hdr;
@@ -534,8 +540,9 @@ int net_init_bridge(const NetClientOptions *opts, const char *name,
 
     helper = bridge->has_helper ? bridge->helper : DEFAULT_BRIDGE_HELPER;
     br     = bridge->has_br     ? bridge->br     : DEFAULT_BRIDGE_INTERFACE;
+    ifname = bridge->has_ifname ? bridge->ifname : DEFAULT_BRIDGE_IFNAME;
 
-    fd = net_bridge_run_helper(helper, br);
+    fd = net_bridge_run_helper(helper, br, ifname);
     if (fd == -1) {
         return -1;
     }
@@ -686,11 +693,12 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
     const char *script = NULL; /* suppress wrong "uninit'd use" gcc warning */
     const char *downscript = NULL;
     const char *vhostfdname;
+    const char *br;
     char ifname[128];
 
     assert(opts->kind == NET_CLIENT_OPTIONS_KIND_TAP);
     tap = opts->tap;
-    queues = tap->has_queues ? tap->queues : 1;
+    queues      = tap->has_queues  ? tap->queues  : 1;
     vhostfdname = tap->has_vhostfd ? tap->vhostfd : NULL;
 
     /* QEMU vlans does not support multiqueue tap, in this case peer is set.
@@ -775,8 +783,7 @@ int net_init_tap(const NetClientOptions *opts, const char *name,
                          "queues=, and fds= are invalid with helper=");
             return -1;
         }
-
-        fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE);
+        fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE, NULL);
         if (fd == -1) {
             return -1;
         }
diff --git a/qapi-schema.json b/qapi-schema.json
index fdaa9da..700e781 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2676,6 +2676,7 @@
 { 'type': 'NetdevBridgeOptions',
   'data': {
     '*br':     'str',
+    '*ifname': 'str',
     '*helper': 'str' } }
 
 ##
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 287bfd5..39486b1 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -67,7 +67,7 @@ typedef QSIMPLEQ_HEAD(ACLList, ACLRule) ACLList;
 static void usage(void)
 {
     fprintf(stderr,
-            "Usage: qemu-bridge-helper [--use-vnet] --br=bridge --fd=unixfd\n");
+            "Usage: qemu-bridge-helper [--use-vnet] [--ifname=name] --br=bridge --fd=unixfd\n");
 }
 
 static int parse_acl_file(const char *filename, ACLList *acl_list)
@@ -233,6 +233,7 @@ int main(int argc, char **argv)
     int use_vnet = 0;
     int mtu;
     const char *bridge = NULL;
+    const char *ifname = NULL;
     char iface[IFNAMSIZ];
     int index;
     ACLRule *acl_rule;
@@ -259,6 +260,8 @@ int main(int argc, char **argv)
             bridge = &argv[index][5];
         } else if (strncmp(argv[index], "--fd=", 5) == 0) {
             unixfd = atoi(&argv[index][5]);
+        } else if (strncmp(argv[index], "--ifname=", 9) == 0) {
+            ifname = &argv[index][9];
         } else {
             usage();
             return EXIT_FAILURE;
@@ -330,7 +333,12 @@ int main(int argc, char **argv)
 
     /* request a tap device, disable PI, and add vnet header support if
      * requested and it's available. */
-    prep_ifreq(&ifr, "tap%d");
+    if (ifname) {
+      /* prep_ifreq will truncate the name if it is too long */
+      prep_ifreq(&ifr, ifname);
+    } else {
+      prep_ifreq(&ifr, "tap%d");
+    }
     ifr.ifr_flags = IFF_TAP|IFF_NO_PI;
     if (use_vnet && has_vnet_hdr(fd)) {
         ifr.ifr_flags |= IFF_VNET_HDR;
-- 
1.8.1.5


 --
Alexandre Kandalintsev

^ permalink raw reply related	[flat|nested] 8+ messages in thread