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@linux.vnet.ibm.com Gerrit-Reviewer: Antoni Segura Puimedon asegurap@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com