Ondřej Svoboda has posted comments on this change. Change subject: networkTests: stop NetworkManager from managing devices using NM_CONTROLLED=no ......................................................................
Patch Set 19: (4 comments) https://gerrit.ovirt.org/#/c/37037/19/tests/functional/dummy.py File tests/functional/dummy.py: Line 51: linkDel(dummy_name) Line 52: except IPRoute2Error as e: Line 53: raise SkipTest("Unable to delete the dummy interface %s: %s" % Line 54: (dummy_name, e)) Line 55: if nm_restore_control: > if the link is removed, why does nm needs to unmanage it? This patch is quite old so NM's behaviour has changed over the time. Now I can only say we're being nice to its runtime database. Whatever we modify, it's good to clean up. I'll try to give you a better answer together with verification. Line 56: networkmanager.toggle_managed(dummy_name, True) Line 57: Line 58: Line 59: @contextmanager https://gerrit.ovirt.org/#/c/37037/19/tests/functional/networkmanager.py File tests/functional/networkmanager.py: Line 47: """ Line 48: if pgrep('NetworkManager'): Line 49: ifcfg_path = NET_CONF_PREF + iface Line 50: with open(ifcfg_path, 'w') as ifcfg: Line 51: ifcfg.write('# Temporary file generated by VDSM. Makes' > just curious here: is there a nmcli api for ignoring connections (devices)? If I were aware of a nmcli command I would have used it instead of needlessly bringing ifcfg files into the game. I would go directly to NM's D-Bus interface (or a binding thereof) if we're to use its API (but let's save it as a next step, possibly paving a way for a configurator). Line 52: ' NetworkManager ignore the interface.\n' Line 53: 'DEVICE={0}\n' Line 54: 'NM_CONTROLLED={1}\n'.format(iface, 'yes' if Line 55: nm_controlled else 'no')) Line 59: raise SkipTest('Failed to set NetworkManager control over %s ' Line 60: 'to %s.\n%s\n%s' % (iface, nm_controlled, out, Line 61: err)) Line 62: finally: Line 63: rmFile(ifcfg_path) > isn't the file removed too early? or it should exist just for the life of t No, the file's existence is meant to be short-lived, just to pass a message to NetworkManager. For both managing and unmanaging I want to create a file and discard it right after use. I recall that nmcli would refuse to load a file from a location other than /etc/sysconfig/network-scripts/ (for whatever, security reasons?), it's pretty picky about its input. I haven't tried to pipe into it but I believe it's not possible. https://gerrit.ovirt.org/#/c/37037/19/tests/functional/veth.py File tests/functional/veth.py: Line 45: except IPRoute2Error: Line 46: raise SkipTest('Failed to create a veth pair.') Line 47: finally: Line 48: # the peer device is removed by the kernel Line 49: dummy.remove(left_side, nm_restore_control=False) > this try-finally block be changed to 'with vethIf()'? vethIf is gone, and we're actually inside its descendant, just becoming more beefed up :-) Line 50: Line 51: Line 52: setIP = dummy.setIP Line 53: setIP -- To view, visit https://gerrit.ovirt.org/37037 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icc8ea9e0b0d410203ed3045e735c9450eee316ae Gerrit-PatchSet: 19 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ido Barkan <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Ondřej Svoboda <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
