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

2014-01-23 Thread William Dauchy
On Mon, Jan 20, 2014 at 10:23 PM, Alexandre Kandalintsev
s...@messir.net wrote:
 I was worried that the amount of changes would
 turn the maintainers away from the patch.

 Another problem is that I think there is little demand
 for this patch.

 But let's try to push it once again. Give me a week or
 two and I'll submit a new version.

ack; please cc me when re-submitting.

Thanks,

-- 
William



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

2014-01-20 Thread Alexandre Kandalintsev
Hi William,

I was worried that the amount of changes would
turn the maintainers away from the patch.

Another problem is that I think there is little demand
for this patch.

But let's try to push it once again. Give me a week or
two and I'll submit a new version.

--
best regards,
Alexandre


 Hi Alexandre,
 
 On Mon, Mar 25, 2013 at 10:28 PM, Alexandre Kandalintsev
 s...@messir.net wrote:
  Ok, lets go this way. We will define patterns in bridge.conf like
  ~~~
  allowifname vm*
  ~~~
 
 Do you have any news about this patch?
 
 Regards,



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

2014-01-14 Thread William Dauchy
Hi Alexandre,

On Mon, Mar 25, 2013 at 10:28 PM, Alexandre Kandalintsev
s...@messir.net wrote:
 Ok, lets go this way. We will define patterns in bridge.conf like
 ~~~
 allowifname vm*
 ~~~

Do you have any news about this patch?

Regards,
-- 
William



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

2013-03-25 Thread Stefan Hajnoczi
On Thu, Mar 21, 2013 at 07:05:09PM +0100, Alexandre Kandalintsev wrote:
 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?

qemu-bridge-helper is a setuid root binary.  It allows access to things
an unprivileged user normally cannot do.  We need to be very careful
that new features cannot be abused.

There needs to be a policy in qemu-bridge-helper to control network
interface naming.

Imagine an existing qemu-bridge-helper deployment.  Now if your patch is
merged and the new qemu-bridge-helper is installed, unprivileged users
can create arbitrarily named network interfaces.

It was previously not possible to create arbitrarily named network
interfaces.  This might pose a security problem given firewall
configuration, monitoring software, etc which isn't configured to deal
with these new interface names.

By default, custom names should not be allowed.  Perhaps the
qemu-bridge-helper configuration file needs an option to specify a glob
pattern, e.g. vm*.

This way the host system administrator can restrict network interface
names while still allowing humand-friendly names.

Stefan



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

2013-03-25 Thread Alexandre Kandalintsev
 By default, custom names should not be allowed.  Perhaps the
 qemu-bridge-helper configuration file needs an option to specify a
 glob pattern, e.g. vm*.
 
 This way the host system administrator can restrict network interface
 names while still allowing humand-friendly names.

Ok, lets go this way. We will define patterns in bridge.conf like
~~~
allowifname vm*
~~~

--
Alexandre Kandalintsev



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

2013-03-21 Thread Alexandre Kandalintsev
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 s...@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 s...@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 = 

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

2013-03-21 Thread Eric Blake
On 03/21/2013 12:05 PM, Alexandre Kandalintsev wrote:
 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.
 

 +++ b/qapi-schema.json
 @@ -2676,6 +2676,7 @@
  { 'type': 'NetdevBridgeOptions',
'data': {
  '*br': 'str',
 +'*ifname': 'str',
  '*helper': 'str' } }

You also need to add a line documenting this field:

# @ifname: #optional Set the interface name that will be used (since 1.5).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature