Dan Kenigsberg has posted comments on this change.
Change subject: Don't fail silently when ifup fails.
......................................................................
Patch Set 7: I would prefer that you didn't submit this
(4 inline comments)
I'd suggest to solve the synchronous use case, and try not to make the async
case any worse. In a later patch, we can consider how the async case should be
solved (I don't think we can do much about it :-( )
....................................................
File vdsm/configNetwork.py
Line 72: # In /etc/sysconfig/network-scripts/ifup* the last line
usually
Line 73: # contains the error reason.
Line 74: raise ConfigNetworkError(ne.ERR_FAILED_IFUP, out[-1])
Line 75: elif toCall is not None:
Line 76: toCall()
our async ifup is dangerous on its own right - we cannot tell if and when we'd
receive an IP address. Adding a callback on top of this, to be called in an
undeterministic timing, would only make it worse. I'd hate to see a libvirt
network appearing out of the blue.
Line 77: return rc, out, err
Line 78:
Line 79: if async:
Line 80: # wait for dhcp in another thread,
Line 934:
Line 935: nic = nics[0] if nics else None
Line 936: iface = bonding or nic
Line 937: blockingDhcp = utils.tobool(options.get('blockingdhcp'))
Line 938: async = options.get('bootproto') == 'dhcp' and not blockingDhcp
this is defined up here, and used so much down the road...
Line 939:
Line 940: # take down nics that need to be changed
Line 941: vlanedIfaces = [v['iface'] for v in _netinfo.vlans.values()]
Line 942: if bonding not in vlanedIfaces:
Line 1013: ifup(iface, vlanBootproto == 'dhcp' and not blockingDhcp,
Line 1014: libvirtNetworkCreation)
Line 1015:
Line 1016: if bridged:
Line 1017: ifup(network, bridgeBootproto == 'dhcp' and not blockingDhcp,
I do not think this patch would solve the cited bug: in the bug, we had a
static IP given to the host, that had a collision with another host on the same
LAN. static IP is set synchronously, where the rc is returned by ifup(), then
ignored here.
Line 1018: libvirtNetworkCreation)
Line 1019:
Line 1020: # add libvirt network
Line 1021: if not async:
....................................................
File vdsm/neterrors.py
Line 26: ERR_BAD_BONDING = 25
Line 27: ERR_BAD_VLAN = 26
Line 28: ERR_BAD_BRIDGE = 27
Line 29: ERR_USED_BRIDGE = 28
Line 30: ERR_FAILED_IFUP = 30
if you go outside 20-29, you should update the comment in define.py
--
To view, visit http://gerrit.ovirt.org/8415
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1cc9dcc0a6b55d36fc937e1d364bd9c256ecd70a
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Igor Lvovsky <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
