Martin Polednik has posted comments on this change. Change subject: vm: graphics: fix console open after restore ......................................................................
Patch Set 1: Code-Review-1 (4 comments) I'm not exactly happy with that the data is stored in devices/data, but the test itself is in vmTests.py. Would it make sense to bootstrap devices/creation(?)/graphics.py at this point, or do we wait? Seems fine mostly, just pointing small tweaks :) https://gerrit.ovirt.org/#/c/44842/1/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1956: return parsedSrcDomXML.toxml() Line 1957: Line 1958: def _correctSpiceConfiguration(self, srcDomXML): Line 1959: """ Line 1960: TODO: documentation :( Is there any reason not to add it now? Line 1961: """ Line 1962: outDomXML = srcDomXML Line 1963: for dev in self._devices[hwclass.GRAPHICS]: Line 1964: outDomXML = dev.updateXML(outDomXML) Line 1958: def _correctSpiceConfiguration(self, srcDomXML): Line 1959: """ Line 1960: TODO: documentation Line 1961: """ Line 1962: outDomXML = srcDomXML Is it needed to create a new reference in this case? Couldn't you operate directly on the srcDomXML (asking because I'm not sure how mutable the structure is at this point)? Line 1963: for dev in self._devices[hwclass.GRAPHICS]: Line 1964: outDomXML = dev.updateXML(outDomXML) Line 1965: return outDomXML Line 1966: https://gerrit.ovirt.org/#/c/44842/1/vdsm/virt/vmdevices/graphics.py File vdsm/virt/vmdevices/graphics.py: Line 18: # Refer to the README and COPYING files for full details of the license Line 19: # Line 20: Line 21: import logging Line 22: import xml.etree.ElementTree as etree awesome! Line 23: Line 24: import libvirt Line 25: Line 26: from vdsm import netinfo Line 137: if not utils.tobool(self.specParams.get('disableTicketing', False)): Line 138: attrs['passwd'] = '*****' Line 139: attrs['passwdValidTo'] = '1970-01-01T00:00:01' Line 140: Line 141: def updateXML(self, domXML): > obviously I'm not proud nor happy about this bruteforce, but proper fix nee I dislike the name - it's very generic while the functionality is quite specific.Id' say something like reset_spice_password (with lowerCamelCase or whatever is more consistent) would be more explanatory. Line 142: if self.device == 'spice': Line 143: domObj = etree.fromstring(domXML) Line 144: devices = domObj.findall('devices') Line 145: -- 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: 1 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: 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
