All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
@ 2016-09-13  7:11 Alexey Kardashevskiy
  2016-09-13 14:49 ` Eric Blake
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Alexey Kardashevskiy @ 2016-09-13  7:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Alexey Kardashevskiy, Eric Blake

The tap backend is already using qemu-bridge-helper to attach tap
interface to a bridge but (unlike the bridge backend) it always uses
the default bridge name - br0.

This adds a "br" property support to the tap backend.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
--
Changes:
v2:
* documented a new member in json and hx
---
 net/tap.c        |  4 +++-
 qapi-schema.json |  3 +++
 qemu-options.hx  | 12 +++++++-----
 3 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/net/tap.c b/net/tap.c
index 6abb962..b6896a7 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -857,7 +857,9 @@ free_fail:
             return -1;
         }
 
-        fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE,
+        fd = net_bridge_run_helper(tap->helper,
+                                   tap->has_br ?
+                                   tap->br : DEFAULT_BRIDGE_INTERFACE,
                                    errp);
         if (fd == -1) {
             return -1;
diff --git a/qapi-schema.json b/qapi-schema.json
index c4f3674..dbb9f8f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2575,6 +2575,8 @@
 #
 # @downscript: #optional script to shut down the interface
 #
+# @br: #optional bridge name
+#
 # @helper: #optional command to execute to configure bridge
 #
 # @sndbuf: #optional send buffer limit. Understands [TGMKkb] suffixes.
@@ -2604,6 +2606,7 @@
     '*fds':        'str',
     '*script':     'str',
     '*downscript': 'str',
+    '*br':         'str',
     '*helper':     'str',
     '*sndbuf':     'size',
     '*vnet_hdr':   'bool',
diff --git a/qemu-options.hx b/qemu-options.hx
index a71aaf8..0b3ea42 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1594,10 +1594,11 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "                configure a host TAP network backend with ID 'str'\n"
 #else
     "-netdev tap,id=str[,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile]\n"
-    "         [,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
+    "         [,br=bridge][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
     "         [,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
     "         [,poll-us=n]\n"
     "                configure a host TAP network backend with ID 'str'\n"
+    "                connected to a bridge (default=" DEFAULT_BRIDGE_INTERFACE ")\n"
     "                use network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
     "                to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
     "                to deconfigure it\n"
@@ -1884,8 +1885,8 @@ processed and applied to -net user. Mixing them with the new configuration
 syntax gives undefined results. Their use for new applications is discouraged
 as they will be removed from future versions.
 
-@item -netdev tap,id=@var{id}[,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,helper=@var{helper}]
-@itemx -net tap[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,helper=@var{helper}]
+@item -netdev tap,id=@var{id}[,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,br=@var{bridge}][,helper=@var{helper}]
+@itemx -net tap[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,br=@var{bridge}][,helper=@var{helper}]
 Connect the host TAP network interface @var{name} to VLAN @var{n}.
 
 Use the network script @var{file} to configure it and the network script
@@ -1896,8 +1897,9 @@ automatically provides one. The default network configure script is
 to disable script execution.
 
 If running QEMU as an unprivileged user, use the network helper
-@var{helper} to configure the TAP interface. The default network
-helper executable is @file{/path/to/qemu-bridge-helper}.
+@var{helper} to configure the TAP interface and attach it to the bridge.
+The default network helper executable is @file{/path/to/qemu-bridge-helper}
+and the default bridge device is @file{br0}.
 
 @option{fd}=@var{h} can be used to specify the handle of an already
 opened host TAP interface.
-- 
2.5.0.rc3

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-13  7:11 [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge Alexey Kardashevskiy
@ 2016-09-13 14:49 ` Eric Blake
  2016-09-13 15:39   ` Greg Kurz
  2016-09-14 14:17 ` Greg Kurz
  2016-09-14 21:04 ` Paolo Bonzini
  2 siblings, 1 reply; 12+ messages in thread
From: Eric Blake @ 2016-09-13 14:49 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]

On 09/13/2016 02:11 AM, Alexey Kardashevskiy wrote:
> The tap backend is already using qemu-bridge-helper to attach tap
> interface to a bridge but (unlike the bridge backend) it always uses
> the default bridge name - br0.
> 
> This adds a "br" property support to the tap backend.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> --
> Changes:
> v2:
> * documented a new member in json and hx
> ---
>  net/tap.c        |  4 +++-
>  qapi-schema.json |  3 +++
>  qemu-options.hx  | 12 +++++++-----
>  3 files changed, 13 insertions(+), 6 deletions(-)
> 

> +++ b/qapi-schema.json
> @@ -2575,6 +2575,8 @@
>  #
>  # @downscript: #optional script to shut down the interface
>  #
> +# @br: #optional bridge name

Missing a '(since 2.8)' designator.

Also, we don't have to abbreviate; 'bridge-name' may be easier to
understand than 'br', as well as a mention of the default value if the
parameter is not supplied.

> +#
>  # @helper: #optional command to execute to configure bridge
>  #
>  # @sndbuf: #optional send buffer limit. Understands [TGMKkb] suffixes.
> @@ -2604,6 +2606,7 @@
>      '*fds':        'str',
>      '*script':     'str',
>      '*downscript': 'str',
> +    '*br':         'str',
>      '*helper':     'str',
>      '*sndbuf':     'size',
>      '*vnet_hdr':   'bool',

Oh, we already use underscore, so if you go with a longer name,
'bridge_name' would be more consistent than 'bridge-name', even though
we prefer dash over underscore in new interfaces.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-13 14:49 ` Eric Blake
@ 2016-09-13 15:39   ` Greg Kurz
  2016-09-14  4:23     ` Alexey Kardashevskiy
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Kurz @ 2016-09-13 15:39 UTC (permalink / raw)
  To: Eric Blake; +Cc: Alexey Kardashevskiy, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1805 bytes --]

On Tue, 13 Sep 2016 09:49:09 -0500
Eric Blake <eblake@redhat.com> wrote:

> On 09/13/2016 02:11 AM, Alexey Kardashevskiy wrote:
> > The tap backend is already using qemu-bridge-helper to attach tap
> > interface to a bridge but (unlike the bridge backend) it always uses
> > the default bridge name - br0.
> > 
> > This adds a "br" property support to the tap backend.
> > 
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > --
> > Changes:
> > v2:
> > * documented a new member in json and hx
> > ---
> >  net/tap.c        |  4 +++-
> >  qapi-schema.json |  3 +++
> >  qemu-options.hx  | 12 +++++++-----
> >  3 files changed, 13 insertions(+), 6 deletions(-)
> >   
> 
> > +++ b/qapi-schema.json
> > @@ -2575,6 +2575,8 @@
> >  #
> >  # @downscript: #optional script to shut down the interface
> >  #
> > +# @br: #optional bridge name  
> 
> Missing a '(since 2.8)' designator.
> 
> Also, we don't have to abbreviate; 'bridge-name' may be easier to
> understand than 'br', as well as a mention of the default value if the
> parameter is not supplied.
> 

FWIW @br is consistent with what we already have in NetdevBridgeOptions
since 1.2.

> > +#
> >  # @helper: #optional command to execute to configure bridge
> >  #
> >  # @sndbuf: #optional send buffer limit. Understands [TGMKkb] suffixes.
> > @@ -2604,6 +2606,7 @@
> >      '*fds':        'str',
> >      '*script':     'str',
> >      '*downscript': 'str',
> > +    '*br':         'str',
> >      '*helper':     'str',
> >      '*sndbuf':     'size',
> >      '*vnet_hdr':   'bool',  
> 
> Oh, we already use underscore, so if you go with a longer name,
> 'bridge_name' would be more consistent than 'bridge-name', even though
> we prefer dash over underscore in new interfaces.
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-13 15:39   ` Greg Kurz
@ 2016-09-14  4:23     ` Alexey Kardashevskiy
  2016-09-14 13:54       ` Eric Blake
  0 siblings, 1 reply; 12+ messages in thread
From: Alexey Kardashevskiy @ 2016-09-14  4:23 UTC (permalink / raw)
  To: Greg Kurz, Eric Blake; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1555 bytes --]

On 14/09/16 01:39, Greg Kurz wrote:
> On Tue, 13 Sep 2016 09:49:09 -0500
> Eric Blake <eblake@redhat.com> wrote:
> 
>> On 09/13/2016 02:11 AM, Alexey Kardashevskiy wrote:
>>> The tap backend is already using qemu-bridge-helper to attach tap
>>> interface to a bridge but (unlike the bridge backend) it always uses
>>> the default bridge name - br0.
>>>
>>> This adds a "br" property support to the tap backend.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> --
>>> Changes:
>>> v2:
>>> * documented a new member in json and hx
>>> ---
>>>  net/tap.c        |  4 +++-
>>>  qapi-schema.json |  3 +++
>>>  qemu-options.hx  | 12 +++++++-----
>>>  3 files changed, 13 insertions(+), 6 deletions(-)
>>>   
>>
>>> +++ b/qapi-schema.json
>>> @@ -2575,6 +2575,8 @@
>>>  #
>>>  # @downscript: #optional script to shut down the interface
>>>  #
>>> +# @br: #optional bridge name  
>>
>> Missing a '(since 2.8)' designator.
>>
>> Also, we don't have to abbreviate; 'bridge-name' may be easier to
>> understand than 'br', as well as a mention of the default value if the
>> parameter is not supplied.


The existing NetdevBridgeOptions does not do this in json as well and it
does in qemu-options.hx, so does my patch. Do I have to fix a bridge first
before reposting this patch?...

When exactly is the content of qapi-schema.json's comments shown to a user?


>>
> 
> FWIW @br is consistent with what we already have in NetdevBridgeOptions
> since 1.2.

Right, this is why "br=".


-- 
Alexey


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-14  4:23     ` Alexey Kardashevskiy
@ 2016-09-14 13:54       ` Eric Blake
  0 siblings, 0 replies; 12+ messages in thread
From: Eric Blake @ 2016-09-14 13:54 UTC (permalink / raw)
  To: Alexey Kardashevskiy, Greg Kurz; +Cc: qemu-devel

[-- Attachment #1: Type: text/plain, Size: 1117 bytes --]

On 09/13/2016 11:23 PM, Alexey Kardashevskiy wrote:

>>>
>>> Also, we don't have to abbreviate; 'bridge-name' may be easier to
>>> understand than 'br', as well as a mention of the default value if the
>>> parameter is not supplied.
> 
> 
> The existing NetdevBridgeOptions does not do this in json as well and it
> does in qemu-options.hx, so does my patch. Do I have to fix a bridge first
> before reposting this patch?...

If that interface is already baked in as 'br', then leave it alone.

> 
> When exactly is the content of qapi-schema.json's comments shown to a user?

Not yet, but Marc-Andre has patches pending that merge qemu-options.hx
and qemu-schema.json so that we have a single file with documentation,
rather than two semi-inconsistent copies.

> 
> 
>>>
>>
>> FWIW @br is consistent with what we already have in NetdevBridgeOptions
>> since 1.2.
> 
> Right, this is why "br=".

Okay, the argument of consistency with pre-existing interfaces is
reasonable.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-13  7:11 [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge Alexey Kardashevskiy
  2016-09-13 14:49 ` Eric Blake
@ 2016-09-14 14:17 ` Greg Kurz
  2016-09-14 15:12   ` Eric Blake
  2016-09-14 21:04 ` Paolo Bonzini
  2 siblings, 1 reply; 12+ messages in thread
From: Greg Kurz @ 2016-09-14 14:17 UTC (permalink / raw)
  To: Alexey Kardashevskiy; +Cc: qemu-devel, Jason Wang

On Tue, 13 Sep 2016 17:11:54 +1000
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> The tap backend is already using qemu-bridge-helper to attach tap
> interface to a bridge but (unlike the bridge backend) it always uses
> the default bridge name - br0.
> 
> This adds a "br" property support to the tap backend.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> --

Cool ! This allows to easily attach to virbr0 :)

Now that a consensus seems to have been reached for @br:

Reviewed-by: Greg Kurz <groug@kaod.org>
Tested-by: Greg Kurz <groug@kaod.org>

Also Cc'ing the tap device maintainer.

Cheers.

--
Greg

> Changes:
> v2:
> * documented a new member in json and hx
> ---
>  net/tap.c        |  4 +++-
>  qapi-schema.json |  3 +++
>  qemu-options.hx  | 12 +++++++-----
>  3 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/net/tap.c b/net/tap.c
> index 6abb962..b6896a7 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -857,7 +857,9 @@ free_fail:
>              return -1;
>          }
>  
> -        fd = net_bridge_run_helper(tap->helper, DEFAULT_BRIDGE_INTERFACE,
> +        fd = net_bridge_run_helper(tap->helper,
> +                                   tap->has_br ?
> +                                   tap->br : DEFAULT_BRIDGE_INTERFACE,
>                                     errp);
>          if (fd == -1) {
>              return -1;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index c4f3674..dbb9f8f 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2575,6 +2575,8 @@
>  #
>  # @downscript: #optional script to shut down the interface
>  #
> +# @br: #optional bridge name
> +#
>  # @helper: #optional command to execute to configure bridge
>  #
>  # @sndbuf: #optional send buffer limit. Understands [TGMKkb] suffixes.
> @@ -2604,6 +2606,7 @@
>      '*fds':        'str',
>      '*script':     'str',
>      '*downscript': 'str',
> +    '*br':         'str',
>      '*helper':     'str',
>      '*sndbuf':     'size',
>      '*vnet_hdr':   'bool',
> diff --git a/qemu-options.hx b/qemu-options.hx
> index a71aaf8..0b3ea42 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -1594,10 +1594,11 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
>      "                configure a host TAP network backend with ID 'str'\n"
>  #else
>      "-netdev tap,id=str[,fd=h][,fds=x:y:...:z][,ifname=name][,script=file][,downscript=dfile]\n"
> -    "         [,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
> +    "         [,br=bridge][,helper=helper][,sndbuf=nbytes][,vnet_hdr=on|off][,vhost=on|off]\n"
>      "         [,vhostfd=h][,vhostfds=x:y:...:z][,vhostforce=on|off][,queues=n]\n"
>      "         [,poll-us=n]\n"
>      "                configure a host TAP network backend with ID 'str'\n"
> +    "                connected to a bridge (default=" DEFAULT_BRIDGE_INTERFACE ")\n"
>      "                use network scripts 'file' (default=" DEFAULT_NETWORK_SCRIPT ")\n"
>      "                to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
>      "                to deconfigure it\n"
> @@ -1884,8 +1885,8 @@ processed and applied to -net user. Mixing them with the new configuration
>  syntax gives undefined results. Their use for new applications is discouraged
>  as they will be removed from future versions.
>  
> -@item -netdev tap,id=@var{id}[,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,helper=@var{helper}]
> -@itemx -net tap[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,helper=@var{helper}]
> +@item -netdev tap,id=@var{id}[,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,br=@var{bridge}][,helper=@var{helper}]
> +@itemx -net tap[,vlan=@var{n}][,name=@var{name}][,fd=@var{h}][,ifname=@var{name}][,script=@var{file}][,downscript=@var{dfile}][,br=@var{bridge}][,helper=@var{helper}]
>  Connect the host TAP network interface @var{name} to VLAN @var{n}.
>  
>  Use the network script @var{file} to configure it and the network script
> @@ -1896,8 +1897,9 @@ automatically provides one. The default network configure script is
>  to disable script execution.
>  
>  If running QEMU as an unprivileged user, use the network helper
> -@var{helper} to configure the TAP interface. The default network
> -helper executable is @file{/path/to/qemu-bridge-helper}.
> +@var{helper} to configure the TAP interface and attach it to the bridge.
> +The default network helper executable is @file{/path/to/qemu-bridge-helper}
> +and the default bridge device is @file{br0}.
>  
>  @option{fd}=@var{h} can be used to specify the handle of an already
>  opened host TAP interface.

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-14 14:17 ` Greg Kurz
@ 2016-09-14 15:12   ` Eric Blake
  2016-09-22  7:03     ` Jason Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Eric Blake @ 2016-09-14 15:12 UTC (permalink / raw)
  To: Greg Kurz, Alexey Kardashevskiy; +Cc: Jason Wang, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 997 bytes --]

On 09/14/2016 09:17 AM, Greg Kurz wrote:
> On Tue, 13 Sep 2016 17:11:54 +1000
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
>> The tap backend is already using qemu-bridge-helper to attach tap
>> interface to a bridge but (unlike the bridge backend) it always uses
>> the default bridge name - br0.
>>
>> This adds a "br" property support to the tap backend.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> --
> 
> Cool ! This allows to easily attach to virbr0 :)
> 
> Now that a consensus seems to have been reached for @br:

Yes, but there was still my other comment that:


>> +++ b/qapi-schema.json
>> @@ -2575,6 +2575,8 @@
>>  #
>>  # @downscript: #optional script to shut down the interface
>>  #
>> +# @br: #optional bridge name

this is missing a '(since 2.8)' designator.  Perhaps the maintainer can
supply it without needing a v3.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-13  7:11 [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge Alexey Kardashevskiy
  2016-09-13 14:49 ` Eric Blake
  2016-09-14 14:17 ` Greg Kurz
@ 2016-09-14 21:04 ` Paolo Bonzini
  2016-09-19  0:33   ` Alexey Kardashevskiy
  2 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2016-09-14 21:04 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel



On 13/09/2016 09:11, Alexey Kardashevskiy wrote:
> The tap backend is already using qemu-bridge-helper to attach tap
> interface to a bridge but (unlike the bridge backend) it always uses
> the default bridge name - br0.
> 
> This adds a "br" property support to the tap backend.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Stupid question ahead: how does -netdev bridge compare to -netdev tap
after this patch?  Is there a case left where you must use -netdev bridge?

Or can we make -netdev bridge a synonym for "-netdev
tap,helper=/default/path/to/helper"?

Thanks,

Paolo

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-14 21:04 ` Paolo Bonzini
@ 2016-09-19  0:33   ` Alexey Kardashevskiy
  2016-09-19 11:59     ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Alexey Kardashevskiy @ 2016-09-19  0:33 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel

On 15/09/16 07:04, Paolo Bonzini wrote:
> 
> 
> On 13/09/2016 09:11, Alexey Kardashevskiy wrote:
>> The tap backend is already using qemu-bridge-helper to attach tap
>> interface to a bridge but (unlike the bridge backend) it always uses
>> the default bridge name - br0.
>>
>> This adds a "br" property support to the tap backend.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> 
> Stupid question ahead: how does -netdev bridge compare to -netdev tap
> after this patch?  Is there a case left where you must use -netdev bridge?
> 
> Or can we make -netdev bridge a synonym for "-netdev
> tap,helper=/default/path/to/helper"?

I looked through history but I could not understand why "bridge" was
introduced in the first place as even there (a7c36ee4920ea) is an example of

-netdev tap,helper="/usr/local/libexec/qemu-bridge-helper --br=qemubr0",id=hn0

so it was assumed even then that people might want tap on a specific bridge.

So my stupid question is - what do I have to do to get this accepted
(besides a note that it is 2.8+) or it is not interesting to anyone? :)


-- 
Alexey

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-19  0:33   ` Alexey Kardashevskiy
@ 2016-09-19 11:59     ` Paolo Bonzini
  2016-09-22  7:05       ` Jason Wang
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2016-09-19 11:59 UTC (permalink / raw)
  To: Alexey Kardashevskiy, qemu-devel



On 19/09/2016 02:33, Alexey Kardashevskiy wrote:
> On 15/09/16 07:04, Paolo Bonzini wrote:
>>
>>
>> On 13/09/2016 09:11, Alexey Kardashevskiy wrote:
>>> The tap backend is already using qemu-bridge-helper to attach tap
>>> interface to a bridge but (unlike the bridge backend) it always uses
>>> the default bridge name - br0.
>>>
>>> This adds a "br" property support to the tap backend.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>
>> Stupid question ahead: how does -netdev bridge compare to -netdev tap
>> after this patch?  Is there a case left where you must use -netdev bridge?
>>
>> Or can we make -netdev bridge a synonym for "-netdev
>> tap,helper=/default/path/to/helper"?
> 
> I looked through history but I could not understand why "bridge" was
> introduced in the first place as even there (a7c36ee4920ea) is an example of
> 
> -netdev tap,helper="/usr/local/libexec/qemu-bridge-helper --br=qemubr0",id=hn0
> 
> so it was assumed even then that people might want tap on a specific bridge.
> 
> So my stupid question is - what do I have to do to get this accepted
> (besides a note that it is 2.8+) or it is not interesting to anyone? :)

I think the patch is even more interesting because it lets us simplify
the code for -netdev bridge.

Paolo

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-14 15:12   ` Eric Blake
@ 2016-09-22  7:03     ` Jason Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Wang @ 2016-09-22  7:03 UTC (permalink / raw)
  To: Eric Blake, Greg Kurz, Alexey Kardashevskiy; +Cc: qemu-devel



On 2016年09月14日 23:12, Eric Blake wrote:
> On 09/14/2016 09:17 AM, Greg Kurz wrote:
>> On Tue, 13 Sep 2016 17:11:54 +1000
>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>>> The tap backend is already using qemu-bridge-helper to attach tap
>>> interface to a bridge but (unlike the bridge backend) it always uses
>>> the default bridge name - br0.
>>>
>>> This adds a "br" property support to the tap backend.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> --
>> Cool ! This allows to easily attach to virbr0 :)
>>
>> Now that a consensus seems to have been reached for @br:
> Yes, but there was still my other comment that:
>
>
>>> +++ b/qapi-schema.json
>>> @@ -2575,6 +2575,8 @@
>>>   #
>>>   # @downscript: #optional script to shut down the interface
>>>   #
>>> +# @br: #optional bridge name
> this is missing a '(since 2.8)' designator.  Perhaps the maintainer can
> supply it without needing a v3.
>

Adding "since 2.8" and applied.

Thanks

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

* Re: [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge
  2016-09-19 11:59     ` Paolo Bonzini
@ 2016-09-22  7:05       ` Jason Wang
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Wang @ 2016-09-22  7:05 UTC (permalink / raw)
  To: Paolo Bonzini, Alexey Kardashevskiy, qemu-devel



On 2016年09月19日 19:59, Paolo Bonzini wrote:
>
> On 19/09/2016 02:33, Alexey Kardashevskiy wrote:
>> On 15/09/16 07:04, Paolo Bonzini wrote:
>>>
>>> On 13/09/2016 09:11, Alexey Kardashevskiy wrote:
>>>> The tap backend is already using qemu-bridge-helper to attach tap
>>>> interface to a bridge but (unlike the bridge backend) it always uses
>>>> the default bridge name - br0.
>>>>
>>>> This adds a "br" property support to the tap backend.
>>>>
>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> Stupid question ahead: how does -netdev bridge compare to -netdev tap
>>> after this patch?  Is there a case left where you must use -netdev bridge?
>>>
>>> Or can we make -netdev bridge a synonym for "-netdev
>>> tap,helper=/default/path/to/helper"?
>> I looked through history but I could not understand why "bridge" was
>> introduced in the first place as even there (a7c36ee4920ea) is an example of
>>
>> -netdev tap,helper="/usr/local/libexec/qemu-bridge-helper --br=qemubr0",id=hn0
>>
>> so it was assumed even then that people might want tap on a specific bridge.
>>
>> So my stupid question is - what do I have to do to get this accepted
>> (besides a note that it is 2.8+) or it is not interesting to anyone? :)
> I think the patch is even more interesting because it lets us simplify
> the code for -netdev bridge.
>
> Paolo
>

According to http://wiki.qemu.org/Features-Done/HelperNetworking. Looks 
like this allows creating a third party backend without modifying qemu. 
And the bridge helper is an example for this.

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

end of thread, other threads:[~2016-09-22  7:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-13  7:11 [Qemu-devel] [PATCH qemu v2] tap: Allow specifying a bridge Alexey Kardashevskiy
2016-09-13 14:49 ` Eric Blake
2016-09-13 15:39   ` Greg Kurz
2016-09-14  4:23     ` Alexey Kardashevskiy
2016-09-14 13:54       ` Eric Blake
2016-09-14 14:17 ` Greg Kurz
2016-09-14 15:12   ` Eric Blake
2016-09-22  7:03     ` Jason Wang
2016-09-14 21:04 ` Paolo Bonzini
2016-09-19  0:33   ` Alexey Kardashevskiy
2016-09-19 11:59     ` Paolo Bonzini
2016-09-22  7:05       ` Jason Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.