diff mbox series

[v1] docs: remove bridge-utils from requirements

Message ID 20200909104849.22700-1-olaf@aepfle.de (mailing list archive)
State New, archived
Headers show
Series [v1] docs: remove bridge-utils from requirements | expand

Commit Message

Olaf Hering Sept. 9, 2020, 10:48 a.m. UTC
Having the latest Xen, but an obsolete iproute package, is an unusual
combination.

Signed-off-by: Olaf Hering <olaf@aepfle.de>
---
 README | 1 -
 1 file changed, 1 deletion(-)

Comments

Julien Grall Sept. 9, 2020, 12:43 p.m. UTC | #1
Hi,

On 09/09/2020 11:48, Olaf Hering wrote:
> Having the latest Xen, but an obsolete iproute package, is an unusual
> combination.

The README suggests that bridge-utils is required for /sbin/brctl. On 
Debian bullseyes:

42sh> sudo apt-file search brctl
bash-completion: /usr/share/bash-completion/completions/brctl
bridge-utils: /sbin/brctl
bridge-utils: /usr/share/man/man8/brctl.8.gz
python3-networking-mlnx: /usr/bin/ebrctl
python3-networking-mlnx: 
/usr/lib/python3/dist-packages/networking_mlnx/eswitchd/cli/ebrctl.py
zsh-common: /usr/share/zsh/functions/Completion/Linux/_brctl

A grep in the tools directory seems to list some uf ose brctl in the 
hotplug scripts. So can you expand how this is an unusual combination?

Cheers,
Olaf Hering Sept. 9, 2020, 12:52 p.m. UTC | #2
Am Wed, 9 Sep 2020 13:43:10 +0100
schrieb Julien Grall <julien@xen.org>:

> So can you expand how this is an unusual combination?

'ip' is the tool of choice since a couple of years. What 'git grep' shows is just compat code.

Olaf
Julien Grall Sept. 9, 2020, 1:06 p.m. UTC | #3
On 09/09/2020 13:52, Olaf Hering wrote:
> Am Wed, 9 Sep 2020 13:43:10 +0100
> schrieb Julien Grall <julien@xen.org>:
> 
>> So can you expand how this is an unusual combination?
> 
> 'ip' is the tool of choice since a couple of years. What 'git grep' shows is just compat code.

Right. I think we want to keep bridge-utils in the README until the 
compat code is removed. So a better suggestion would be to mention which 
version of 'iproute' is enough to avoid install bridge-utils. How about:

"bridge-utils (if iroute version < ...)"

Cheers,
Olaf Hering Sept. 9, 2020, 1:17 p.m. UTC | #4
Am Wed, 9 Sep 2020 14:06:42 +0100
schrieb Julien Grall <julien@xen.org>:

> "bridge-utils (if iroute version < ...)"

A brief search in git://git.kernel.org/pub/scm/network/iproute2/iproute2.git shows bridge support appeared in v3.5.0 from 2012.

One can barely run Xen on such old dists, so the patch is fine as it is.


Olaf
Jan Beulich Sept. 9, 2020, 1:22 p.m. UTC | #5
On 09.09.2020 15:17, Olaf Hering wrote:
> Am Wed, 9 Sep 2020 14:06:42 +0100
> schrieb Julien Grall <julien@xen.org>:
> 
>> "bridge-utils (if iroute version < ...)"
> 
> A brief search in git://git.kernel.org/pub/scm/network/iproute2/iproute2.git shows bridge support appeared in v3.5.0 from 2012.
> 
> One can barely run Xen on such old dists, so the patch is fine as it is.

I'm still at least smoke testing Xen every now and then even on
two SLE10 hosts I have (mainly for backporting work), fwiw.

Jan
Julien Grall Sept. 9, 2020, 1:43 p.m. UTC | #6
Hi,

On 09/09/2020 14:17, Olaf Hering wrote:
> Am Wed, 9 Sep 2020 14:06:42 +0100
> schrieb Julien Grall <julien@xen.org>:
> 
>> "bridge-utils (if iroute version < ...)"
> 
> A brief search in git://git.kernel.org/pub/scm/network/iproute2/iproute2.git shows bridge support appeared in v3.5.0 from 2012.
> 
> One can barely run Xen on such old dists, so the patch is fine as it is.
IMHO, the README is not only here to explain the required softwares for 
the latest distribution. It is also here to explain all the dependencies 
regardless whether Xen can be barely run or not...

At the moment, brctl is still a dependency as hotplug scripts will use 
it if present. In fact, they will *only* fallback to iproute if brctl is 
not present:

     if which brctl >&/dev/null; then
         bridge=$(brctl show | awk 'NR==2{print$1}')
     else
         bridge=$(bridge link | cut -d" " -f7)
     fi

If you think that bridge-utils should be dropped, then please send a 
proposal to remove/deprecate brctl. Until then I think we ought to 
mention the dependency in the README.

Cheers,
Olaf Hering Sept. 9, 2020, 1:54 p.m. UTC | #7
Am Wed, 9 Sep 2020 14:43:28 +0100
schrieb Julien Grall <julien@xen.org>:

> If you think that bridge-utils should be dropped, then please send a 
> proposal to remove/deprecate brctl.

This was already done with 0e7c69bd3c0b35a677d73843b39522787ccf5a3f.

The current code is just the simple form of a fallback, it does not represent the fact that brctl is the preferred tool. 'ip' is most likely always present, but finding its capabilities is probably cumbersome.


Olaf
Julien Grall Sept. 9, 2020, 2:18 p.m. UTC | #8
Hi,

On 09/09/2020 14:54, Olaf Hering wrote:
> Am Wed, 9 Sep 2020 14:43:28 +0100
> schrieb Julien Grall <julien@xen.org>:
> 
>> If you think that bridge-utils should be dropped, then please send a
>> proposal to remove/deprecate brctl.
> 
> This was already done with 0e7c69bd3c0b35a677d73843b39522787ccf5a3f.
> 
> The current code is just the simple form of a fallback, it does not represent the fact that brctl is the preferred tool. 'ip' is most likely always present, but finding its capabilities is probably cumbersome.

Apologies, it was a mistake to suggest that "deprecating" would be 
enough to remove from the README. This was actually against my initial 
comment:

"IMHO, the README is not only here to explain the required softwares for 
the latest distribution. It is also here to explain all the dependencies 
regardless whether Xen can be barely run or not..."

Cheers,
diff mbox series

Patch

diff --git a/README b/README
index 0e4787c1a6..ce580d3029 100644
--- a/README
+++ b/README
@@ -57,7 +57,6 @@  provided by your OS distributor:
     * Development install of GLib v2.0 (e.g. libglib2.0-dev)
     * Development install of Pixman (e.g. libpixman-1-dev)
     * pkg-config
-    * bridge-utils package (/sbin/brctl)
     * iproute package (/sbin/ip)
     * GNU bison and GNU flex
     * GNU gettext