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 <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Frank Kobzik <[email protected]> Gerrit-Reviewer: Martin Polednik <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
