Francesco Romani has posted comments on this change. Change subject: virt: graphdev: add the GraphicsDevice class ......................................................................
Patch Set 21: (5 comments) http://gerrit.ovirt.org/#/c/26895/21/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 1387: graphicsAttrs['keymap'] = self.specParams['keyMap'] Line 1388: Line 1389: graphics = XMLElement('graphics', **graphicsAttrs) Line 1390: Line 1391: # handle deprecated channel name in a smart way, > add "TODO" string (I do not see added smartness). Right, I just forgot, and I'll fix ASAP. Line 1392: # not just chop 1st char! Line 1393: if graphicsAttrs['type'] == 'spice': Line 1394: if self.specParams.get('spiceSecureChannels'): Line 1395: for chan in self.specParams['spiceSecureChannels'].split(','): Line 1862: Normalize graphics device provided by conf. Line 1863: """ Line 1864: return [{ Line 1865: 'type': GRAPHICS_DEVICES, Line 1866: 'device': 'spice' if self.conf.get('display', 'qxl') == 'qxl' > if you want to drop support for qxlnc (and I won't mind that, if you verify Actually here I forgot about qxlnc, which I'll to remove in a future separate patch. Line 1867: else 'vnc', Line 1868: 'specParams': dict( Line 1869: (v, self.conf[k]) Line 1870: for k, v in GraphicsDevice.LEGACY_MAPPING.iteritems() Line 2482: params['devices'] = newDevices Line 2483: params.update(dispParams) Line 2484: return params Line 2485: Line 2486: def getDisplayParams(self, dev): > used once, and does not use "self". better be declared as private staticmet Will move inside getRemoteMachneParams. Line 2487: """ Line 2488: rebuild display* parameters as old engine would send them. Line 2489: symmetric to getConfGraphics. Line 2490: This is needed because all the configuration VDSM should accept Line 2493: way to achieve this goal. Line 2494: """ Line 2495: params = { Line 2496: 'display': 'qxl' if dev['device'] == 'spice' else dev['device']} Line 2497: for k, v in GraphicsDevice.LEGACY_MAPPING.iteritems(): > renaming (k, v) to (legacyName, newName) would make this function more read Agreed and done Line 2498: try: Line 2499: params[k] = dev['specParams'][v] Line 2500: except KeyError: Line 2501: # just go ahead: either no specParams or no param specified Line 2497: for k, v in GraphicsDevice.LEGACY_MAPPING.iteritems(): Line 2498: try: Line 2499: params[k] = dev['specParams'][v] Line 2500: except KeyError: Line 2501: # just go ahead: either no specParams or no param specified > My credo does not allow me to accept this. A missing 'specParams' in a grap I adhere to this credo. Changed as you suggested. Line 2502: pass Line 2503: return params Line 2504: Line 2505: def getStats(self): -- To view, visit http://gerrit.ovirt.org/26895 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I82e4414aa3efe7c756ba8ef9b7b47e591613a717 Gerrit-PatchSet: 21 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Martin Polednik <mpole...@redhat.com> Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com> Gerrit-Reviewer: Vinzenz Feenstra <vfeen...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
