Francesco Romani has posted comments on this change.
Change subject: virt: graphdev: add the GraphicsDevice class
......................................................................
Patch Set 24:
(7 comments)
http://gerrit.ovirt.org/#/c/26895/24/vdsm/API.py
File vdsm/API.py:
Line 199: except:
Line 200: self.log.error("Error restoring VM parameters",
Line 201: exc_info=True)
Line 202:
Line 203: requiredParams = ['vmId', 'memSize']
To maintain the same behavior (error reporting), shouldn't you
have a check
Yes, but this requires a fair amount of duplication of what we'll
do in buildConfDevices. I'll save this until we decide what to do with the support for
VM with no graphics devices/display.
Line 204: for param in requiredParams:
Line 205: if param not in vmParams:
Line 206: self.log.error('Missing required parameter %s' %
(param))
Line 207: return {'status': {'code':
errCode['MissParam']
http://gerrit.ovirt.org/#/c/26895/24/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 1395: if not utils.tobool(self.specParams.get('disableTicketing',
'')):
I think that False would be better than '' considering
utils.tobool would r
Agreed, will fix.
Line 4848: self.conf['devices'].append(diskDev)
Line 4849:
Line 4850: def _getUnderlyingGraphicsDeviceInfo(self):
Line 4851: """
Line 4852: Obtain graphics framebuffer devices info from libvirt.
s/graphics/graphic/
Done
Line 4853: Libvirt only allows 0 or 1 device of each type, so mapping between
Line 4854: _devices and conf.devices is based on this fact.
Line 4855: The libvirt docs are a bit sparse on this topic, but as in libvirt
Line 4856: 1.2.3 if one tries to create a domain with 2+ SPICE graphics devices,
Line 4849:
Line 4850: def _getUnderlyingGraphicsDeviceInfo(self):
Line 4851: """
Line 4852: Obtain graphics framebuffer devices info from libvirt.
Line 4853: Libvirt only allows 0 or 1 device of each type, so mapping between
s/only allows 0 or 1/allows at most one/
Done
Line 4854: _devices and conf.devices is based on this fact.
Line 4855: The libvirt docs are a bit sparse on this topic, but as in libvirt
Line 4856: 1.2.3 if one tries to create a domain with 2+ SPICE graphics devices,
Line 4857: virsh responds: error: unsupported configuration: only 1 graphics
Line 4851: """
Line 4852: Obtain graphics framebuffer devices info from libvirt.
Line 4853: Libvirt only allows 0 or 1 device of each type, so mapping between
Line 4854: _devices and conf.devices is based on this fact.
Line 4855: The libvirt docs are a bit sparse on this topic, but as in libvirt
s/as in/as of/
Done
Line 4856: 1.2.3 if one tries to create a domain with 2+ SPICE graphics devices,
Line 4857: virsh responds: error: unsupported configuration: only 1 graphics
Line 4858: device of each type (sdl, vnc, spice) is supported
Line 4859: """
Line 4867: if dev.get('device') == graphicsType:
Line 4868: port = gxml.getAttribute('port')
Line 4869: if port:
Line 4870: dev['port'] = port
Line 4871: port = gxml.getAttribute('tlsPort')
better not to reuse the local variable port, I'd rather have
tlsport as a v
Done
Line 4872: if port:
Line 4873: dev['tlsPort'] = port
Line 4874: self._updateLegacyConf(dev)
Line 4875:
Line 5070: try:
Line 5071: nets = netinfo.networks()
Line 5072: device = nets[network].get('iface', network)
Line 5073: ip = netinfo.getaddr(device)
Line 5074: except:
no naked excepts allowed ;-)
Indeed. Will fix.
Line 5075: ip = config.get('addresses', 'guests_gateway_ip')
Line 5076: if ip == '':
Line 5077: ip = '0'
Line 5078: logging.info('network %s: using %s', network, ip)
--
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: 24
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Frank Kobzik <fkobzik(a)redhat.com>
Gerrit-Reviewer: Martin Polednik <mpoledni(a)redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skrivanek(a)redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes