Dan Kenigsberg has uploaded a new change for review. Change subject: Do not update timestamp on API.getVMList ......................................................................
Do not update timestamp on API.getVMList Commit 95302a moved the call to _updateTimestamp from the XMLRPC binding to the API level. This has the adverse effect of having a local API call, that does not use networking at all, to update the client timestamp. Since MoM calls getVMList periodically, this effectively breaks the connectivityCheck feature of the setupNetworks and editNetwork API calls: even if such a call breaks network configuration, Vdsm thinks that it is still reachable by a client, and maintains the broken configuration instead of reverting it. Bug-Url: https://bugzilla.redhat.com/1119024 Change-Id: I9f8d4a6de9f79d91c667e577934fdaa2b762beb1 Signed-off-by: Dan Kenigsberg <[email protected]> Reviewed-on: http://gerrit.ovirt.org/30178 Reviewed-by: Piotr Kliczewski <[email protected]> Reviewed-by: Antoni Segura Puimedon <[email protected]> --- M tests/functional/networkTests.py M vdsm/API.py M vdsm/rpc/BindingXMLRPC.py M vdsm/rpc/Bridge.py 4 files changed, 13 insertions(+), 5 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/56/30256/1 diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index a5ddf4f..b4f4997 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -2080,3 +2080,11 @@ # cleanup status, msg = self.vdsm_net.setupNetworks( {NETWORK_NAME: {'remove': True}}, {}, NOCHK) + + @cleanupNet + def testSetupNetworksConnectivityCheck(self): + status, msg = self.vdsm_net.setupNetworks( + {NETWORK_NAME: {'bridged': True}}, {}, + {'connectivityCheck': True, 'connectivityTimeout': 0.1}) + self.assertEqual(status, errors.ERR_LOST_CONNECTION) + self.assertNetworkDoesntExist(NETWORK_NAME) diff --git a/vdsm/API.py b/vdsm/API.py index ace68d1..f328a0b 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -65,7 +65,7 @@ USER_SHUTDOWN_MESSAGE = 'System going down' -def _updateTimestamp(): +def updateTimestamp(): # The setup+editNetwork API uses this log file to # determine if this host is still accessible. We use a # file (rather than an event) because setup+editNetwork is @@ -1192,7 +1192,7 @@ def ping(self): "Ping the server. Useful for tests" - _updateTimestamp() + updateTimestamp() return {'status': doneCode} def getCapabilities(self): @@ -1200,7 +1200,7 @@ Report host capabilities. """ hooks.before_get_caps() - _updateTimestamp() # required for some ovirt-3.0.z Engines + updateTimestamp() # required for some ovirt-3.0.z Engines c = caps.get() c['netConfigDirty'] = str(self._cif._netConfigDirty) c = hooks.after_get_caps(c) @@ -1310,8 +1310,6 @@ return d else: return {'vmId': d['vmId'], 'status': d['status']} - - _updateTimestamp() # required for editNetwork flow # To improve complexity, convert 'vms' to set(vms) vmSet = set(vmList) diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py index 525fcc0..8775273 100644 --- a/vdsm/rpc/BindingXMLRPC.py +++ b/vdsm/rpc/BindingXMLRPC.py @@ -334,6 +334,7 @@ return vm.create(vmParams) def getVMList(self, fullStatus=False, vmList=()): + API.updateTimestamp() # required for editNetwork flow api = API.Global() return api.getVMList(fullStatus, vmList) diff --git a/vdsm/rpc/Bridge.py b/vdsm/rpc/Bridge.py index 4246116..b83f22d 100644 --- a/vdsm/rpc/Bridge.py +++ b/vdsm/rpc/Bridge.py @@ -283,6 +283,7 @@ This call is only interested in returning the VM UUIDs so pass False for the first argument in order to suppress verbose results. """ + API.updateTimestamp() # required for editNetwork flow vmList = args.get('vmList', []) return API.Global().getVMList(False, vmList) -- To view, visit http://gerrit.ovirt.org/30256 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9f8d4a6de9f79d91c667e577934fdaa2b762beb1 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Dan Kenigsberg <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
