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@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