Wed, Aug 22, 2018 at 09:54:58AM CEST, olichtne(a)redhat.com wrote:
On Tue, Aug 21, 2018 at 03:00:01PM +0200, Jan Tluka wrote:
> Tue, Aug 21, 2018 at 02:11:06PM CEST, csfakian(a)redhat.com wrote:
> >+ m1.bond0 = BondDevice(mode="active-backup",
name="my_bond0")
> >+ m1.eth0.down()
> >+ m1.eth1.down()
> >+ m1.bond0.slave_add(m1.eth0)
> >+ m1.bond0.slave_add(m1.eth1)
>
> Could we enhance the abstraction here? Probably we could completely remove:
> m1.eth0.down()
> m1.eth1.down()
>
> and move it into BondDevice class when calling
> m1.bond0.slave_add(m1.eth0)
This is more of a generic "next" branch question... it's an interesting
idea, but I'm not 100% convinced... the slave_{add, del} methods are
shared by all devices inheriting from MasterDevice class. At the same
time the rules to adding slaves can be slightly different based on the
type of device, some types don't require first setting the slaves down,
some do, some require the master to be down.
We could redefine the method of BondDevice,
dev.down()
super(BondDevice, self).slave_add(dev)
Hiding this inside the slave_{add, del} methods could become
confusing
and give testers false assumptions - e.g. adding a slave while some test
is running will end up setting all other slaves and the master down,
then add the slave and then set everything back up, whereas in the code
it would just look like a slave was added.
I understand your point but I think this goes against easy-to-use.
For bonding there's no other way to enslave a device unless
it's down. You just have to do it otherwise the code throws exception.
And what's the difference of completely avoiding LNST and simply run:
ip l set eth0 down
ip l set eth1 down
echo "+eth0" > /sys/class/net/bond0/bonding/slaves
echo "+eth1" > /sys/class/net/bond0/bonding/slaves
Of course, there's matching algorithm that makes it much more easy.
I just don't see an advantage here.
Let's discuss this upstream on one of our Monday calls.
-Jan
>So I think for now I prefer this to be explicit even if it leads to a
>couple more lines. But we can discuss this on an upstream meeting, see
>what others think...
>
>-Ondrej