Dan Kenigsberg has posted comments on this change.
Change subject: netinfo: Simplify the check if an interface still has any other users.
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(4 inline comments)
sorry, I do not quite understand this patch.
....................................................
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):
the suggested semantics has nothing to do with "Other" users. It simply reports
if at the moment, iface has users. How is it different to the ifaceUsers() function?
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
I think it's better to cast the returned value into boolean. I dislike polymorphic
functions that return list/string/None.
....................................................
File vdsm/configNetwork.py
Line 250
Line 251
Line 252
Line 253
Line 254
Are you sure this has no effect? it seems like a redundant ifdown/ifup cycle, which wastes
time, even for a nic with no ip address.
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()
What do you mean by "temporary change"? Are you removing this in a future
patch?
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>