Francesco Romani has posted comments on this change. Change subject: vm: graphics: fix settings after restore ......................................................................
Patch Set 11: (4 comments) https://gerrit.ovirt.org/#/c/44842/11/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1929: devXml.get('type')) Line 1930: except LookupError: Line 1931: self.log.warning('configuration mismatch: graphic device ' Line 1932: 'type %s found in domain XML, but not among ' Line 1933: 'VM devices' % devXml.get('type')) > read after reading comments below will fix 'graphics'. Line 1934: else: Line 1935: devObj.setupPassword(devXml) Line 1936: return ET.tostring(domObj) Line 1937: Line 2016: return response.error('hotplugNic', e.message) Line 2017: Line 2018: return {'status': doneCode, 'vmList': self.status()} Line 2019: Line 2020: def _lookupDeviceByClass(self, devType, device): > (unsure) does this work for all devices? interface comes to mind probably not. But an explicit _lookupDeviceSomething is better than another for loop buried elsewhere: at least we can find it more easily when we'll do the much needed refactoring. Line 2021: for dev in self._devices[devType][:]: Line 2022: try: Line 2023: if dev.device == device: Line 2024: return dev Line 2024: return dev Line 2025: except AttributeError: Line 2026: continue Line 2027: raise LookupError('Device instance for device identified by class %s ' Line 2028: 'not found' % device) > The error text is quite inaccurate, because 'device' for e.g. hostdev (but We're again short on good names here. But will improve in the direction you suggested. Line 2029: Line 2030: def _lookupDeviceByAlias(self, devType, alias): Line 2031: for dev in self._devices[devType][:]: Line 2032: try: https://gerrit.ovirt.org/#/c/44842/11/vdsm/virt/vmdevices/graphics.py File vdsm/virt/vmdevices/graphics.py: Line 131: graphics.setAttrs(listen='0') Line 132: Line 133: return graphics Line 134: Line 135: def _setPasswd(self, attrs): > Not sure I like the inconsistence in this case :) no issues about setPassword, but I'd like to keep it private. Line 136: if not utils.tobool(self.specParams.get('disableTicketing', False)): Line 137: attrs['passwd'] = '*****' Line 138: attrs['passwdValidTo'] = '1970-01-01T00:00:01' Line 139: -- To view, visit https://gerrit.ovirt.org/44842 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic9f11e2d87cc715c77d305c005c1cd7f502d506d Gerrit-PatchSet: 11 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Michal Skrivanek <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
