Change in vdsm[master]: virt: graphdev: add the GraphicsDevice class

asegurap at redhat.com asegurap at redhat.com
Wed May 14 16:48:21 UTC 2014


Antoni Segura Puimedon has posted comments on this change.

Change subject: virt: graphdev: add the GraphicsDevice class
......................................................................


Patch Set 24:

(6 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 that there is either 'display' or at least on graphic device
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 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/
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/
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/
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 variable name.
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 ;-)
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 at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce 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: Michal Skrivanek <michal.skrivanek at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list