Change in vdsm[master]: vdsm: [wip] modeling graphics as a device

mpoledni at redhat.com mpoledni at redhat.com
Wed Feb 5 12:20:44 UTC 2014


Martin Polednik has posted comments on this change.

Change subject: vdsm: [wip] modeling graphics as a device
......................................................................


Patch Set 4:

(3 comments)

Also try to add functional test for multiple graphics devices configurations.

http://gerrit.ovirt.org/#/c/23555/4/vdsm/vm.py
File vdsm/vm.py:

Line 1175:         commandLine.appendChildWithArgs('qemu:arg', value='-usbdevice')
Line 1176:         commandLine.appendChildWithArgs('qemu:arg', value='keyboard')
Line 1177:         self.dom.appendChild(commandLine)
Line 1178: 
Line 1179:     def appendGraphicsFromDisplay(self):
Try to remove this function alltogether and handle legacy graphics device in getConfGraphics / GraphicsDevice.
Line 1180:         """
Line 1181:         Legacy add graphics section to domain xml.
Line 1182: 
Line 1183:         <graphics autoport="yes" listen="0" type="vnc"/>


Line 1339:                   keymap='en-us' passwdValidTo='1970-01-01T00:00:01'>
Line 1340:           <listen type='address' address='0'/>
Line 1341:         </graphics>
Line 1342: 
Line 1343:         """
I'd rather see log messages consistent with other VDSM parts, accessible by some keyword for searching. Also, self.log.debug seems to be more appropriate.
Line 1344:         self.log.error("=========== Using new GraphicsDevice")
Line 1345:         pprint(self.conf)
Line 1346:         self.log.error("*********** ")
Line 1347:         pprint(self.specParams)


Line 2109:             devices[NIC_DEVICES] = self.getConfNetworkInterfaces()
Line 2110:             devices[SOUND_DEVICES] = self.getConfSound()
Line 2111:             devices[VIDEO_DEVICES] = self.getConfVideo()
Line 2112:             # devices[GRAPHICS_DEVICES] = self.getConfVideo() - todo isn't it
Line 2113:             # handled by appendGraphicsFromDisplay??
Currently yes, but in order to be consistent with other VmDevices the handling should be done in getConfGraphics.
Line 2114:             devices[CONTROLLER_DEVICES] = self.getConfController()
Line 2115:         else:
Line 2116:             for dev in self.conf.get('devices'):
Line 2117:                 try:


-- 
To view, visit http://gerrit.ovirt.org/23555
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia59b933f4cd1e3ab562ad2ec1c237007c83f214c
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Frank Kobzik <fkobzik at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkobzik at redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list