[PATCH v1] docs: remove bridge-utils from requirements

Olaf Hering posted 1 patch 3 years, 7 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20200909104849.22700-1-olaf@aepfle.de
Maintainers: Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <iwj@xenproject.org>, Julien Grall <julien@xen.org>, Jan Beulich <jbeulich@suse.com>, Wei Liu <wl@xen.org>, Stefano Stabellini <sstabellini@kernel.org>, George Dunlap <george.dunlap@citrix.com>
README | 1 -
1 file changed, 1 deletion(-)
[PATCH v1] docs: remove bridge-utils from requirements
Posted by Olaf Hering 3 years, 7 months ago
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(-)

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

Re: [PATCH v1] docs: remove bridge-utils from requirements
Posted by Julien Grall 3 years, 7 months ago
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,

-- 
Julien Grall

Re: [PATCH v1] docs: remove bridge-utils from requirements
Posted by Olaf Hering 3 years, 7 months ago
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
Re: [PATCH v1] docs: remove bridge-utils from requirements
Posted by Julien Grall 3 years, 7 months ago

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,

-- 
Julien Grall

Re: [PATCH v1] docs: remove bridge-utils from requirements
Posted by Olaf Hering 3 years, 7 months ago
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
Re: [PATCH v1] docs: remove bridge-utils from requirements
Posted by Jan Beulich 3 years, 7 months ago
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

Re: [PATCH v1] docs: remove bridge-utils from requirements
Posted by Julien Grall 3 years, 7 months ago
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,

-- 
Julien Grall

Re: [PATCH v1] docs: remove bridge-utils from requirements
Posted by Olaf Hering 3 years, 7 months ago
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
Re: [PATCH v1] docs: remove bridge-utils from requirements
Posted by Julien Grall 3 years, 7 months ago
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,

-- 
Julien Grall