Mark Wu has posted comments on this change.
Change subject: netinfo: Simplify the check if an interface still has any other users.
......................................................................
Patch Set 1: (4 inline comments)
....................................................
File lib/vdsm/netinfo.py
Line 604: if iface == vdict['iface']:
Line 605: users.add(v)
Line 606: return users
Line 607:
Line 608: def ifaceHasOtherUsers(self, iface):
ifaceUsers return all the current users of an interface: possible cases are:
1. bridge
2. bond
3. one or more vlans
4. zero or more vlans + bridgeless
And ifaceUsers only check case 3 and 4, because bridge and bond use the interface
exclusively. It's impossible that a bridge or bond is remained after other user is
removed.
But I am fine to just use ifaceUsers instead of ifaceOtherUsers because it's easy to
understand and robust of any out of sync case.
So is it ifaceUsers good for you?
Line 609: """
Line 610: It's used to check if an interface has other direct users when one
of
Line 611: its user is removed. It could only happen in the following two cases:
Line 612:
Line 622: \
Line 623: --> network3 (bridgeless)
Line 624:
Line 625: """
Line 626: return (list(self.getVlansForIface(iface)) or
Done
....................................................
File vdsm/configNetwork.py
Line 250
Line 251
Line 252
Line 253
Line 254
Sorry for the misleading words. let me recap it.
1. users of an interface = user is removed just now + other users
2. here it uses an updated netinfo instance, so the 'user is removed just now '
is not the result of 'ifaceUsers', but nicOtherUsers always discard it whether or
not the 'user is removed just now ' is in the set. The remained set after
discarding an element not in the set and discarding an element in the set are the same.
3. So the old implementation can work with the netinfo instance just before removal and
the netinfo snapshot when the user is removed just before this check
4. I didn't change the old behavior, so not related to the ifdown/ipup stuff.
Line 344: ifup(iface)
Line 345:
Line 346: # The (relatively) new setupNetwork verb allows to remove a network
Line 347: # defined on top of an bonding device without break the bond itself.
Line 348: _netinfo = netinfo.NetInfo()
Yep, in
http://gerrit.ovirt.org/#/c/14873/, only there's one usage of
ifaceHasOtherUsers
Line 349: if implicitBonding:
Line 350: if bonding and not _netinfo.ifaceHasOtherUsers(bonding):
Line 351: ifdown(bonding)
Line 352: configWriter.removeBonding(bonding)
--
To view, visit
http://gerrit.ovirt.org/15416
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie70cbfcc4c561c98f2e90685329900796517f933
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>