Nir Soffer has posted comments on this change. Change subject: virt: Don't fail when existingConnAction is unset for a SPICE device ......................................................................
Patch Set 2: (3 comments) https://gerrit.ovirt.org/#/c/56836/2//COMMIT_MSG Commit Message: Line 16: Line 17: This should be fixed on the Engine side. One approach may be not to Line 18: send the parameter at all when updating the SPICE ticket. However, Vdsm Line 19: crashes in such a case. This patch fixes Vdsm problems when the Line 20: parameter is not present, making future changes in Engine possible. This approach sounds much better, engine is in control now. Is this parameter documented in the schema? Line 21: Line 22: Change-Id: I2fde83ceb994eaafd2aa956d1fa82ce6cb16094c Line 23: Bug-Url: https://bugzilla.redhat.com/1060573 https://gerrit.ovirt.org/#/c/56836/2/tests/vmOperationsTests.py File tests/vmOperationsTests.py: Line 128: testvm.setDownStatus(exitCode, vmexitreason.GENERIC_ERROR) Line 129: self.assertEqual(testvm.getStats()['timeOffset'], Line 130: str(self.BASE_OFFSET + self.UPDATE_OFFSETS[-1])) Line 131: Line 132: @permutations([['disconnect'], [None]]) What about "keep"? Line 133: def testUpdateSingleDeviceGraphics(self, connected): Line 134: if connected: Line 135: graphics_params = _GRAPHICS_DEVICE_PARAMS Line 136: connected_attribute = ' connected="disconnect"' Line 136: connected_attribute = ' connected="disconnect"' Line 137: else: Line 138: graphics_params = {k: v for k, v in _GRAPHICS_DEVICE_PARAMS.items() Line 139: if k != 'existingConnAction'} Line 140: connected_attribute = '' This is little complicated, and these tests are already too complicated. Why not add new test for each case without any logic? Line 141: devXmls = ( Line 142: '<graphics%s passwd="12345678"' Line 143: ' port="5900" type="spice"/>' % (connected_attribute,), Line 144: '<graphics passwd="12345678" port="5900" type="vnc"/>') -- To view, visit https://gerrit.ovirt.org/56836 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2fde83ceb994eaafd2aa956d1fa82ce6cb16094c Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Milan Zamazal <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
