Re: [Qemu-devel] [PATCH] tap: add the possibility to specify a tap prefix

2014-01-14 Thread Paolo Bonzini 
Il 14/01/2014 18:15, William Dauchy ha scritto:
 this will permit to specify an interface prefix to the tap instead of the
 default one (tap)
 this functionnality is useful when you need an easy way to find the
 interfaces attached to a given virtual machine
 
 example:
  -net bridge,prefix=tapvmA. -net bridge,prefix=tapvmA.
  will create `tapvmA.0` and `tapvmA.1`
  `brctl show | grep vmA` will be an easy way to find the interfaces
  attached to the vmA
 
 Signed-off-by: will...@gandi.net

I think this was nacked already in the past.  You would need to
implement some kind of ACL system like the one that is in place for
bridges.  Without it, for example, you could hijack existing iptables rules.

Sorry for the bad news. :)

Paolo

[Qemu-devel] [PATCH] tap: add the possibility to specify a tap prefix

2014-01-14 Thread William Dauchy 
this will permit to specify an interface prefix to the tap instead of the
default one (tap)
this functionnality is useful when you need an easy way to find the
interfaces attached to a given virtual machine

example:
 -net bridge,prefix=tapvmA. -net bridge,prefix=tapvmA.
 will create `tapvmA.0` and `tapvmA.1`
 `brctl show | grep vmA` will be an easy way to find the interfaces
 attached to the vmA

Signed-off-by: will...@gandi.net
---
 include/net/net.h|  1 +
 net/tap.c| 18 --
 qapi-schema.json |  3 ++-
 qemu-bridge-helper.c |  9 +++--
 qemu-options.hx  |  3 ++-
 5 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 11e1468..4cb0566 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -180,6 +180,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_PREFIX tap
 
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
 
diff --git a/net/tap.c b/net/tap.c
index 39c1cda..667cf17 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -419,7 +419,8 @@ 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 *prefix)
 {
 sigset_t oldmask, mask;
 int pid, status;
@@ -441,7 +442,8 @@ 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 helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + 15];
+char pr_buf[6+IFNAMSIZ] = {0};
+char helper_cmd[PATH_MAX + sizeof(fd_buf) + sizeof(br_buf) + 
sizeof(pr_buf) + 15];
 
 for (i = 0; i  open_max; i++) {
 if (i != STDIN_FILENO 
@@ -453,6 +455,7 @@ static int net_bridge_run_helper(const char *helper, const 
char *bridge)
 }
 
 snprintf(fd_buf, sizeof(fd_buf), %s%d, --fd=, sv[1]);
+snprintf(pr_buf, sizeof(br_buf), %s%s, --tap-prefix=, prefix);
 
 if (strrchr(helper, ' ') || strrchr(helper, '\t')) {
 /* assume helper is a command */
@@ -481,6 +484,7 @@ static int net_bridge_run_helper(const char *helper, const 
char *bridge)
 *parg++ = (char *)--use-vnet;
 *parg++ = fd_buf;
 *parg++ = br_buf;
+*parg++ = pr_buf;
 *parg++ = NULL;
 
 execv(helper, args);
@@ -519,7 +523,7 @@ int net_init_bridge(const NetClientOptions *opts, const 
char *name,
 NetClientState *peer)
 {
 const NetdevBridgeOptions *bridge;
-const char *helper, *br;
+const char *helper, *br, *prefix;
 
 TAPState *s;
 int fd, vnet_hdr;
@@ -528,9 +532,10 @@ int net_init_bridge(const NetClientOptions *opts, const 
char *name,
 bridge = opts-bridge;
 
 helper = bridge-has_helper ? bridge-helper : DEFAULT_BRIDGE_HELPER;
+prefix = bridge-has_prefix ? bridge-prefix : DEFAULT_BRIDGE_PREFIX;
 br = bridge-has_br ? bridge-br : DEFAULT_BRIDGE_INTERFACE;
 
-fd = net_bridge_run_helper(helper, br);
+fd = net_bridge_run_helper(helper, br, prefix);
 if (fd == -1) {
 return -1;
 }
@@ -728,7 +733,7 @@ int net_init_tap(const NetClientOptions *opts, const char 
*name,
 tap-has_vnet_hdr || tap-has_helper || tap-has_queues ||
 tap-has_vhostfd) {
 error_report(ifname=, script=, downscript=, vnet_hdr=, 
- helper=, queues=, and vhostfd= 
+ helper=, queues=, and vhostfd=
  are invalid with fds=);
 return -1;
 }
@@ -773,7 +778,8 @@ int net_init_tap(const NetClientOptions *opts, const char 
*name,
 return -1;
 }
 
-fd = net_bridge_run_helper(tap-helper, DEFAULT_BRIDGE_INTERFACE);
+fd = net_bridge_run_helper(tap-helper, DEFAULT_BRIDGE_INTERFACE,
+   DEFAULT_BRIDGE_PREFIX);
 if (fd == -1) {
 return -1;
 }
diff --git a/qapi-schema.json b/qapi-schema.json
index f27c48a..83d8895 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3028,7 +3028,8 @@
 { 'type': 'NetdevBridgeOptions',
   'data': {
 '*br': 'str',
-'*helper': 'str' } }
+'*helper': 'str',
+'*prefix': 'str'} }
 
 ##
 # @NetdevHubPortOptions
diff --git a/qemu-bridge-helper.c b/qemu-bridge-helper.c
index 6a0974e..6eef43b 100644
--- a/qemu-bridge-helper.c
+++ b/qemu-bridge-helper.c
@@ -67,7 +67,8 @@ 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]

Re: [Qemu-devel] [PATCH] tap: add the possibility to specify a tap prefix

2014-01-14 Thread William Dauchy 
Hello Paolo,

Thanks for your quick reply.

On Jan14 18:31, Paolo Bonzini wrote:
 I think this was nacked already in the past.  You would need to
 implement some kind of ACL system like the one that is in place for
 bridges.  Without it, for example, you could hijack existing iptables rules.

I don't get it; this does not change anything to the existing but
permits to change the default tap string prefix.

Am I missing something?

-- 
William


signature.asc
Description: Digital signature

Re: [Qemu-devel] [PATCH] tap: add the possibility to specify a tap prefix

2014-01-14 Thread Eric Blake 
On 01/14/2014 10:15 AM, William Dauchy wrote:
 this will permit to specify an interface prefix to the tap instead of the
 default one (tap)
 this functionnality is useful when you need an easy way to find the

s/functionnality/functionality/

 interfaces attached to a given virtual machine
 

 +++ b/qapi-schema.json
 @@ -3028,7 +3028,8 @@
  { 'type': 'NetdevBridgeOptions',
'data': {
  '*br': 'str',
 -'*helper': 'str' } }
 +'*helper': 'str',
 +'*prefix': 'str'} }

Need to document the new field, including when it was added.  Something
like:

# @NetdevBridgeOptions
#
# Connect a host TAP network interface to a host bridge device.
#
# @br: #optional bridge name
#
# @helper: #optional command to execute to configure bridge
#
# @prefix: #optional prefix to use in naming the bridge, default
#  tap (since 2.0)
#
# Since 1.2

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] tap: add the possibility to specify a tap prefix

2014-01-14 Thread Paolo Bonzini 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

Il 14/01/2014 18:40, William Dauchy ha scritto:
 I think this was nacked already in the past.  You would need to
 implement some kind of ACL system like the one that is in place
 for bridges.  Without it, for example, you could hijack 
 existing iptables rules.
 I don't get it; this does not change anything to the existing but 
 permits to change the default tap string prefix.
 
 Am I missing something?

See http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg04279.html
and http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg02149.html
for previous discussions on this topic.

Paolo
-BEGIN PGP SIGNATURE-
Version: GnuPG v2.0.22 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJS1YDdAAoJEBvWZb6bTYbyS64P/jJZT2FmUEqh1HiRLl5JA41C
zUZSjs6yIPI2PBZTwAQFR8WbFoQd8WHusqZF2DyMsCCcQ/5SN+H70ZdVavkA30T+
SoiJ2KdBdZdSWocMr2VqU7nVVsUEVkYuVVmjAESB0z2uG+Z/BrJM0Y5LxPbqjAJT
3munoW3w0pU2v4v3zzm48W4GlOUQTsp1vqdIsXhKbMO40G+BuM95LiNyn6g+B+i+
G9rbLN3IVjnsGasIcUNGhMVoTaP4p+NufX7NVX1D0H46wVXgmtGjDRfva3EW2qvv
P0WvTG4b1nRC20zXcmznfOrVd4d9XgtABByvkvzeY6Bawzp5ZW7nV31AVX7H7G+7
vG8AdttsgH3/mYN0VwzVAhwlmhxMbB3Ip3AnfCEGSTSPUV1rMsdA3xIiWZb5Kjqc
xCZloSbLI0E1kFrVepoGBq0g81jVHaPM+BHpSUQSTCbuXqCHrgdm+z3kHkqZbpE8
HmAXkIn4ot4+Q3nZ8a0jEXC2ipJsNl9v8zkvPbTai/5/C2j+1aU7oBLXES63K19w
DoUaFdSgHbEXMx/CTcryWjxdBCrOGiilpanObTIT9cfVbm0LfNLS8sZap3ATE5/6
o7OHQlZ1u0s91yikMiex+UkBjXt2mFJmJ/T/LbOI7rxFV+cnQoA1s7ErbvSa9jaC
RIhf6mVXZGAzCLNVdJNz
=PvwW
-END PGP SIGNATURE-

Re: [Qemu-devel] [PATCH] tap: add the possibility to specify a tap prefix

2014-01-14 Thread William Dauchy 
On Jan14 19:24, Paolo Bonzini wrote:
 See http://lists.gnu.org/archive/html/qemu-devel/2013-03/msg04279.html
 and http://lists.gnu.org/archive/html/qemu-devel/2012-10/msg02149.html
 for previous discussions on this topic.

Thanks for pointing me this out, it's now clear.
-- 
William


signature.asc
Description: Digital signature