Francesco Romani has uploaded a new change for review. Change subject: virt: graphdev: support headless VM ...................................................................... virt: graphdev: support headless VM This patch add support for headless VMs, aka VMs without a graphic device. Noteworthy side effects of this patch: * It is now possible to create a VM without any display (aka headless VM), and they are supported. * The input 'display' parameter of the Vm.create API is no longer mandatory * In the API schema, the display* parameters are now marked as optional, even though Engine is expected to still send them in the near/medium term. * setTicket and the internally used _reviveTicket can now fail if they are invoked against a VM without graphic devices. Change-Id: Iafeb0bebfb43c089614127d94c054175c111ce54 Signed-off-by: Francesco Romani <fromani(a)redhat.com&gt; --- M tests/functional/virtTests.py M tests/vmTests.py M vdsm/API.py M vdsm/virt/vm.py M vdsm_api/vdsmapi-schema.json 5 files changed, 117 insertions(+), 72 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/46/27846/1 diff --git a/tests/functional/virtTests.py b/tests/functional/virtTests.py index a074e62..9a25850 100644 --- a/tests/functional/virtTests.py +++ b/tests/functional/virtTests.py @@ -225,6 +225,14 @@ self._waitForStartup(vm, VM_MINIMAL_UPTIME) @requireKVM + def testHeadlessVm(self): + customization = {'vmId': '77777777-ffff-3333-bbbb-222222222222', + 'vmName': 'testHeadlessVm'} + + with RunningVm(self.vdsm, customization) as vm: + self._waitForStartup(vm, VM_MINIMAL_UPTIME) + + @requireKVM @permutations([['localfs'], ['iscsi'], ['nfs']]) def testVmWithStorage(self, backendType): disk = storage.StorageTest() diff --git a/tests/vmTests.py b/tests/vmTests.py index 69d5643..4022dfb 100644 --- a/tests/vmTests.py +++ b/tests/vmTests.py @@ -1077,6 +1077,11 @@ devs = fake.buildConfDevices() self.assertTrue(devs['graphics']) + def testGraphicDeviceHeadless(self): + with FakeVM(self.conf) as fake: + devs = fake.buildConfDevices() + self.assertFalse(devs['graphics']) + def testGraphicsDeviceMixed(self): """ if proper Graphics Devices are supplied, display* params must be diff --git a/vdsm/API.py b/vdsm/API.py index 14e7bae..4747be1 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -200,7 +200,7 @@ self.log.error("Error restoring VM parameters", exc_info=True) - requiredParams = ['vmId', 'memSize', 'display'] + requiredParams = ['vmId', 'memSize'] for param in requiredParams: if param not in vmParams: self.log.error('Missing required parameter %s' % (param)) @@ -252,11 +252,6 @@ 'No space on /tmp?'}} return errCode['createErr'] - if not vm.GraphicsDevice.isSupportedDisplayType(vmParams): - return {'status': {'code': errCode['createErr'] - ['status']['code'], - 'message': 'Unknown display type %s' % - vmParams.get('display')}} if 'nicModel' not in vmParams: vmParams['nicModel'] = config.get('vars', 'nic_model') return self._cif.createVm(vmParams) diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index a547a5e..5d08cea 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -1344,10 +1344,6 @@ 'main', 'display', 'inputs', 'cursor', 'playback', 'record', 'smartcard', 'usbredir') - @staticmethod - def isSupportedDisplayType(vmParams): - return vmParams.get('display') in ('vnc', 'qxl', 'qxlnc') - def __init__(self, conf, log, **kwargs): super(GraphicsDevice, self).__init__(conf, log, **kwargs) self.port = self.LIBVIRT_PORT_AUTOSELECT @@ -1840,8 +1836,8 @@ if len(devices[device]) > 1: raise ValueError("only a single %s device is " "supported" % device) - if len(devices[GRAPHICS_DEVICES]) != 1: - raise ValueError("one graphics device is required") + if len(devices[GRAPHICS_DEVICES]) > 1: + raise ValueError("only one graphics device is supported") def getConfController(self): """ @@ -1866,12 +1862,16 @@ devType = DEFAULT_VIDEOS[self.arch] elif self.hasSpice: devType = 'qxl' + else: + devType = None - monitors = int(self.conf.get('spiceMonitors', '1')) - vram = '65536' if (monitors <= 2) else '32768' - for idx in range(monitors): - vcards.append({'type': VIDEO_DEVICES, 'specParams': {'vram': vram}, - 'device': devType}) + if devType: + monitors = int(self.conf.get('spiceMonitors', '1')) + vram = '65536' if (monitors <= 2) else '32768' + for idx in range(monitors): + vcards.append({'type': VIDEO_DEVICES, + 'specParams': {'vram': vram}, + 'device': devType}) return vcards @@ -1885,13 +1885,18 @@ for oldName, newName in GraphicsDevice.LEGACY_MAP.iteritems() if oldName in conf) - return [{ - 'type': GRAPHICS_DEVICES, - 'device': ( - 'spice' - if self.conf['display'] in ('qxl', 'qxlnc') - else 'vnc'), - 'specParams': makeSpecParams(self.conf)}] + graphDevs = [] + + if 'display' in self.conf: + graphDevs.append({ + 'type': GRAPHICS_DEVICES, + 'device': ( + 'spice' + if self.conf['display'] in ('qxl', 'qxlnc') + else 'vnc'), + 'specParams': makeSpecParams(self.conf)}) + + return graphDevs def getConfSound(self): """ @@ -2520,10 +2525,6 @@ return self._getExitedVmStats() stats = { - 'displayPort': self.conf['displayPort'], - 'displaySecurePort': self.conf['displaySecurePort'], - 'displayType': self.conf['display'], - 'displayIp': self.conf['displayIp'], 'pid': self.conf['pid'], 'vmType': self.conf['vmType'], 'kvmEnable': self._kvmEnable, @@ -2534,6 +2535,8 @@ stats['cdrom'] = self.conf['cdrom'] if 'boot' in self.conf: stats['boot'] = self.conf['boot'] + + stats.update(self._getGraphicsStats()) decStats = {} try: @@ -2602,6 +2605,16 @@ 'exitReason': self.conf['exitReason']} if 'timeOffset' in self.conf: stats['timeOffset'] = self.conf['timeOffset'] + return stats + + def _getGraphicsStats(self): + stats = {} + if 'display' in self.conf: + stats['displayType'] = self.conf['display'] + stats['displayPort'] = self.conf['displayPort'] + stats['displaySecurePort'] = self.conf['displaySecurePort'] + stats['displayIp'] = self.conf['displayIp'] + # else headless VM return stats def isMigrating(self): @@ -4266,8 +4279,13 @@ return {'status': doneCode, 'vmList': self.status()} def setTicket(self, otp, seconds, connAct, params): - graphics = _domParseStr(self._dom.XMLDesc(0)).childNodes[0]. \ - getElementsByTagName('graphics')[0] + try: + graphics = _domParseStr(self._dom.XMLDesc(0)).childNodes[0]. \ + getElementsByTagName('graphics')[0] + except IndexError: + return { + 'status': {'code': errCode['ticketErr']['status']['code'], + 'message': 'no graphics devices configured'}} graphics.setAttribute('passwd', otp) if int(seconds) > 0: validto = time.strftime('%Y-%m-%dT%H:%M:%S', @@ -4288,9 +4306,13 @@ def _reviveTicket(self, newlife): """Revive an existing ticket, if it has expired or about to expire""" - graphics = _domParseStr( - self._dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)). \ - childNodes[0].getElementsByTagName('graphics')[0] + try: + graphics = _domParseStr( + self._dom.XMLDesc(libvirt.VIR_DOMAIN_XML_SECURE)). \ + childNodes[0].getElementsByTagName('graphics')[0] + except IndexError: + self.log.error('no graphics devices configured') + return validto = max(time.strptime(graphics.getAttribute('passwdValidTo'), '%Y-%m-%dT%H:%M:%S'), time.gmtime(time.time() + newlife)) @@ -4855,19 +4877,20 @@ device of each type (sdl, vnc, spice) is supported """ graphicsXml = _domParseStr(self._lastXMLDesc).childNodes[0]. \ - getElementsByTagName('devices')[0].getElementsByTagName('graphics')[0] + getElementsByTagName('devices')[0].getElementsByTagName('graphics') - graphicsType = graphicsXml.getAttribute('type') - for dev in self.conf['devices']: - if dev.get('device') == graphicsType: - port = graphicsXml.getAttribute('port') - if port: - dev['port'] = port - tlsPort = graphicsXml.getAttribute('tlsPort') - if tlsPort: - dev['tlsPort'] = tlsPort - self._updateLegacyConf(dev) - break + for gxml in graphicsXml: + graphicsType = gxml.getAttribute('type') + + for dev in self.conf['devices']: + if dev.get('device') == graphicsType: + port = gxml.getAttribute('port') + if port: + dev['port'] = port + tlsPort = gxml.getAttribute('tlsPort') + if tlsPort: + dev['tlsPort'] = tlsPort + self._updateLegacyConf(dev) def _getUnderlyingNetworkInterfaceInfo(self): """ @@ -5051,11 +5074,12 @@ return True def _initLegacyConf(self): - self.conf['displayPort'] = GraphicsDevice.LIBVIRT_PORT_AUTOSELECT - self.conf['displaySecurePort'] = \ - GraphicsDevice.LIBVIRT_PORT_AUTOSELECT - self.conf['displayIp'] = _getNetworkIp( - self.conf.get('displayNetwork')) + if 'display' in self.conf: + self.conf['displayPort'] = GraphicsDevice.LIBVIRT_PORT_AUTOSELECT + self.conf['displaySecurePort'] = \ + GraphicsDevice.LIBVIRT_PORT_AUTOSELECT + self.conf['displayIp'] = _getNetworkIp( + self.conf.get('displayNetwork')) def _updateLegacyConf(self, dev): self.conf['display'] = 'qxl' if dev['device'] == 'spice' else 'vnc' diff --git a/vdsm_api/vdsmapi-schema.json b/vdsm_api/vdsmapi-schema.json index a9332ee..64d4179 100644 --- a/vdsm_api/vdsmapi-schema.json +++ b/vdsm_api/vdsmapi-schema.json @@ -3175,13 +3175,17 @@ # # @devices: #optional An array of VM devices present # -# @display: The type of display +# @display: #optional The type of display +# (made optional in version 4.15.0) # -# @displayIp: The IP address to use for accessing the VM display +# @displayIp: #optional The IP address to use for accessing the VM display +# (made optional in version 4.15.0) # -# @displayPort: The port in use for unencrypted display data +# @displayPort: #optional The port in use for unencrypted display data +# (made optional in version 4.15.0) # -# @displaySecurePort: The port in use for encrypted display data +# @displaySecurePort: #optional The port in use for encrypted display data +# (made optional in version 4.15.0) # # @emulatedMachine: #optional The machine specification being emulated # @@ -3235,8 +3239,8 @@ {'type': 'VmDefinition', 'data': {'acpiEnable': 'bool', 'clientIp': 'str', 'cpuShares': 'str', '*cpuType': 'str', '*custom': 'StringMap', '*devices': ['VmDevice'], - 'display': 'VmDisplayType', 'displayIp': 'str', - 'displayPort': 'int', 'displaySecurePort': 'int', + '*display': 'VmDisplayType', '*displayIp': 'str', + '*displayPort': 'int', '*displaySecurePort': 'int', '*emulatedMachine': 'str', '*keyboardLayout': 'str', 'kvmEnable': 'bool', '*maxVCpus': 'uint', 'memSize': 'uint', 'memGuaranteedSize': 'uint', 'nicModel': 'str', 'nice': 'int', @@ -3264,7 +3268,8 @@ # # @devices: #optional An array of VM devices requested # -# @display: The type of display +# @display: #optional The type of display +# (made optional in version 4.15.0) # # @kvmEnable: Indicates if KVM hardware acceleration is enabled # @@ -3300,7 +3305,7 @@ {'type': 'VmParameters', 'data': {'acpiEnable': 'bool', '*bootMenuEnable': 'bool', '*cpuShares': 'str', '*custom': 'StringMap', '*devices': ['VmDevice'], - 'display': 'VmDisplayType', 'kvmEnable': 'bool', 'memSize': 'uint', + '*display': 'VmDisplayType', 'kvmEnable': 'bool', 'memSize': 'uint', 'nice': 'int', 'smp': 'uint', '*smpCoresPerSocket': 'uint', '*smpThreadsPerCore': 'uint', 'timeOffset': 'uint', 'transparentHugePages': 'bool', 'vmId': 'UUID', 'vmName': 'str', @@ -3347,7 +3352,8 @@ # # @devices: An array of VM devices requested # -# @display: The type of display +# @display: #optional The type of display +# (made optional in version 4.15.0) # # @kvmEnable: Indicates if KVM hardware acceleration is enabled # @@ -3372,7 +3378,8 @@ # # @memGuaranteedSize: The amount of memory guaranteed to the VM in MB # -# @displaySecurePort: The port in use for encrypted display data +# @displaySecurePort: #optional The port in use for encrypted display data +# (made optional in version 4.15.0) # # @spiceSecureChannels: Secure space channels comma separated # @@ -3394,11 +3401,13 @@ # @guestFQDN: Fully qualified domain name of the guest OS. (Reported # by the guest agent) # -# @displayIp: The IP address to use for accessing the VM display +# @displayIp: #optional The IP address to use for accessing the VM display +# (made optional in version 4.15.0) # # @keyboardLayout: The keyboard layout string (eg. 'en-us') # -# @displayPort: The port in use for unencrypted display data +# @displayPort: #optional The port in use for unencrypted display data +# (made optional in version 4.15.0) # # @guestIPs: A space separated string of assigned IPv4 addresses # @@ -3416,15 +3425,15 @@ ## {'type': 'VMFullInfo', 'data': {'acpiEnable': 'bool', 'custom': 'StringMap', 'devices': ['VmDevice'], - 'display': 'VmDisplayType', 'kvmEnable': 'bool', 'memSize': 'uint', + '*display': 'VmDisplayType', 'kvmEnable': 'bool', 'memSize': 'uint', 'nice': 'int', 'smp': 'uint', 'smpCoresPerSocket': 'uint', 'timeOffset': 'uint', 'transparentHugePages': 'bool', 'vmId': 'UUID', 'vmName': 'str', 'vmType': 'VmType', 'memGuaranteedSize': 'uint', - 'displaySecurePort': 'uint', 'spiceSecureChannels': 'str', + '*displaySecurePort': 'uint', 'spiceSecureChannels': 'str', 'username': 'str', 'emulatedMachine': 'str', 'pid': 'uint', 'spiceSslCipherSuite': 'str', 'cpuType': 'str', 'pauseCode': 'str', - 'guestFQDN': 'str', 'displayIp': 'str', 'keyboardLayout': 'str', - 'displayPort': 'uint', 'guestIPs': 'str', 'smartcardEnable': 'bool', + 'guestFQDN': 'str', '*displayIp': 'str', 'keyboardLayout': 'str', + '*displayPort': 'uint', 'guestIPs': 'str', 'smartcardEnable': 'bool', 'nicModel': 'VmInterfaceDeviceModel', 'pitReinjection': 'bool', 'status': 'str', 'clientIp': 'str'}} @@ -6016,13 +6025,17 @@ # # Statistics for a running virtual machine. # -# @displayPort: The port in use for unencrypted display data +# @displayPort: #optional The port in use for unencrypted display data +# (made optional in version 4.15.0) # -# @displaySecurePort: The port in use for encrypted display data +# @displaySecurePort: #optional The port in use for encrypted display data +# (made optional in version 4.15.0) # -# @displayType: The type of display in use +# @displayType: #optional The type of display in use +# (made optional in version 4.15.0) # -# @displayIp: The IP address to use for accessing the VM display +# @displayIp: #optional The IP address to use for accessing the VM display +# (made optional in version 4.15.0) # # @pid: The process ID of the underlying qemu process # @@ -6089,8 +6102,8 @@ # Since: 4.10.0 ## {'type': 'RunningVmStats', - 'data': {'displayPort': 'uint', 'displaySecurePort': 'uint', - 'displayType': 'VmDisplayType', 'displayIp': 'str', 'pid': 'uint', + 'data': {'*displayPort': 'uint', '*displaySecurePort': 'uint', + '*displayType': 'VmDisplayType', '*displayIp': 'str', 'pid': 'uint', 'vmType': 'VmType', 'kvmEnable': 'bool', 'network': 'NetworkInterfaceStatsMap', 'disks': 'VmDiskStatsMap', 'monitorResponse': 'int', -- To view, visit http://gerrit.ovirt.org/27846 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iafeb0bebfb43c089614127d94c054175c111ce54 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com&gt;