Change in vdsm[master]: vm: graphics: fix settings after restore
mpolednik at redhat.com
mpolednik at redhat.com
Mon Sep 14 12:47:10 UTC 2015
Martin Polednik has posted comments on this change.
Change subject: vm: graphics: fix settings after restore
......................................................................
Patch Set 8: Code-Review-1
(6 comments)
https://gerrit.ovirt.org/#/c/44842/8//COMMIT_MSG
Commit Message:
Line 6:
Line 7: vm: graphics: fix settings after restore
Line 8:
Line 9: When restoring a VM from hibernation (aka migration from file),
Line 10: if we don't reset the some details of the SPICE configuration.
then what? (weird grammar / missing sentence)
Line 11:
Line 12: This behaviour is easily reproduced on Centos 7.1/2 by:
Line 13: a. booting a VM with SPICE graphic device
Line 14: b. do a snapshot, make sure to check 'snapshot memory'
Line 8:
Line 9: When restoring a VM from hibernation (aka migration from file),
Line 10: if we don't reset the some details of the SPICE configuration.
Line 11:
Line 12: This behaviour is easily reproduced on Centos 7.1/2 by:
Could be omitted due to dependency, nothing major/possibly bad thing though.
Line 13: a. booting a VM with SPICE graphic device
Line 14: b. do a snapshot, make sure to check 'snapshot memory'
Line 15: c. preview the snapshot
Line 16: d. run the VM
Line 9: When restoring a VM from hibernation (aka migration from file),
Line 10: if we don't reset the some details of the SPICE configuration.
Line 11:
Line 12: This behaviour is easily reproduced on Centos 7.1/2 by:
Line 13: a. booting a VM with SPICE graphic device
boot (--img)
Line 14: b. do a snapshot, make sure to check 'snapshot memory'
Line 15: c. preview the snapshot
Line 16: d. run the VM
Line 17: e. check the VM parameters (e.g. qemu)
https://gerrit.ovirt.org/#/c/44842/8/tests/vmTests.py
File tests/vmTests.py:
Line 966: u'type': u'graphics',
Line 967: 'port': u'5900'
Line 968: }
Line 969:
Line 970: with fake.VM(devices=[conf], create_device_objects=True)as testvm:
) as
^
missing space
Line 971: out_dom_xml = testvm._correctSpiceConfiguration(
Line 972: _load_xml('vm_restore_spice_before.xml'))
Line 973:
Line 974: self.assertXMLEqual(out_dom_xml,
https://gerrit.ovirt.org/#/c/44842/8/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1920: """
Line 1921: Fix the configuration of SPICE after resume.
Line 1922: Make sure the ticketing settings are right
Line 1923: """
Line 1924: for dev in self._devices[hwclass.GRAPHICS]:
see graphics.py :(
Line 1925: domXML = dev.setupSpicePassword(domXML)
Line 1926: return domXML
Line 1927:
Line 1928: def _changeDisk(self, diskDeviceXmlElement):
https://gerrit.ovirt.org/#/c/44842/8/vdsm/virt/vmdevices/graphics.py
File vdsm/virt/vmdevices/graphics.py:
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 setupSpicePassword(self, domXML):
This is very sneaky and evil without at least a docstring. The behavior is O(n^2) - for each graphics device, you're iterating all of the graphics devices (similar to getUnderlying*). _correctSpiceConfiguration should only send XML of each device or do all of the setupSpicePassword's in a single pass.
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: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Polednik <mpolednik at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Michal Skrivanek <mskrivan at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list