Re: [Xen-devel] [PATCH] [tools/hotplug] Use ip on systems where brctl is not available

[Steven Haigh <netwiz@xxxxxxxxx>]

On 2019-12-19 02:42, Ian Jackson wrote:
> Steven Haigh writes ("[PATCH] [tools/hotplug] Use ip on systems where
> brctl is not available"):
> > Newer distros like CentOS 8 do not have brctl available. As such, we
> > can't use it to configure networking anymore.
> >
> > This patch will fall back to 'ip' or 'bridge' commands if brctl is not
> > available in the working PATH.
> This looks good to me at least in the brctl case.  I have two minor
> comments.
>
> For the avoidance of doubt, I guess you have tested this in the
> `ip'/`bridge' case ?  How thoroughly ? :-)
I have tested it to the point that it's almost a port of the Fedora 
patch - however the Fedora patch removes brctl completely in favour of 
the ip / bridge commands. While I haven't specifically debugged the 
result on Fedora, the networking works successfully when running a 
Domain-0 in Fedora 31 - which was the source of the 'ip' commands to 
run.
> > -if [ -z "$bridge" ]
> > -then
> > -  bridge=$(brctl show | awk 'NR==2{print$1}')
> > -
> > +if [ -z "$bridge" ]; then
> The presumably-unintentional style change makes the review slightly
> harder...
I'm intending to submit a new patch series after this (to make 
backporting this easier) that cleans up formatting / whitespace / syntax 
across the majority of scripts in the Linux directory. It'll look like a 
hot mess when submitting the next lot of patches - but its better than 
nothing.
> > -    bridge=$(brctl show | cut -d "
> > +    if which brctl >&/dev/null; then
> Maybe introduce
>    have_brctl () { ... }
> so we can say
>    if have_brctl; then
> ?
I don't really have a preference. brctl is used through quite a few 
scripts - none of which really have a standard method of operation or 
common presentation. Some scripts call xen-network-common.sh - some do 
not.
Would I be correct in thinking that your proposal would be to ensure all 
network scripts source xen-network-common.sh - but this would be a more 
invasive change for backporting - hence I've tried to keep it as simple 
as possible for now.
Would a restructure of these things be better for something to be 
committed as yet another patch set (after formatting/style cleanups) 
that makes things a little more consistent?
--
Steven Haigh

? netwiz@xxxxxxxxx     ? https://www.crc.id.au

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel