vdsm-patches May 2014

vdsm-patches@lists.fedorahosted.org

44 participants
645 discussions

Change in vdsm[master]: Added gluster tag support in getAllTasks()
by barumuga＠redhat.com 14 Oct '15

14 Oct '15

Hello Ayal Baron, Timothy Asir, Saggi Mizrahi, Federico Simoncelli, Dan Kenigsberg, I'd like you to do a code review. Please visit http://gerrit.ovirt.org/7579 to review the following change. Change subject: Added gluster tag support in getAllTasks() ...................................................................... Added gluster tag support in getAllTasks() If param tag is empty, all tasks including gluster tasks are returned, else tasks those tags are in param tag list are returned. As below verbs are not consumed by engine/RHS-C yet, its OK to differ in compatibility issue now. glusterVolumeRebalanceStart glusterVolumeRebalanceStatus glusterVolumeReplaceBrickStart glusterVolumeReplaceBrickStatus glusterVolumeRemoveBrickStart glusterVolumeRemoveBrickStatus Change-Id: I9c765cbfebb5ba22f0d21efa04c824ea4daf6432 Signed-off-by: Bala.FA <barumuga(a)redhat.com> --- M tests/gluster_cli_tests.py M vdsm/gluster/cli.py M vdsm/gluster/exception.py M vdsm/storage/taskManager.py 4 files changed, 367 insertions(+), 95 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/79/7579/1 diff --git a/tests/gluster_cli_tests.py b/tests/gluster_cli_tests.py index f442893..9c6357c 100644 --- a/tests/gluster_cli_tests.py +++ b/tests/gluster_cli_tests.py @@ -28,6 +28,7 @@ from gluster import cli as gcli except ImportError: pass +import xml.etree.cElementTree as etree class GlusterCliTests(TestCaseBase): @@ -115,3 +116,74 @@ def test_parsePeerStatus(self): self._parsePeerStatus_empty_test() self._parsePeerStatus_test() + + def _parseVolumeStatusAll_test(self): + out = """<?xml version="1.0" encoding="UTF-8" standalone="yes"?> +<cliOutput> + <opRet>0</opRet> + <opErrno>0</opErrno> + <opErrstr></opErrstr> + <volumes> + <volume> + <name>V1</name> + <id>03eace73-9197-49d0-a877-831bc6e9dac2</id> + <tasks> + <task> + <name>rebalance</name> + <id>12345473-9197-49d0-a877-831bc6e9dac2</id> + </task> + </tasks> + </volume> + <volume> + <name>V2</name> + <id>03eace73-1237-49d0-a877-831bc6e9dac2</id> + <tasks> + <task> + <name>replace-brick</name> + <id>12345473-1237-49d0-a877-831bc6e9dac2</id> + <sourceBrick>192.168.122.167:/tmp/V2-b1</sourceBrick> + <destBrick>192.168.122.168:/tmp/V2-b1</destBrick> + </task> + </tasks> + </volume> + <volume> + <name>V3</name> + <id>03eace73-1237-1230-a877-831bc6e9dac2</id> + <tasks> + <task> + <name>remove-brick</name> + <id>12345473-1237-1230-a877-831bc6e9dac2</id> + <BrickCount>2</BrickCount> + <brick>192.168.122.167:/tmp/V3-b1</brick> + <brick>192.168.122.168:/tmp/V3-b1</brick> + </task> + </tasks> + </volume> + </volumes> +</cliOutput>""" + tree = etree.fromstring(out) + status = gcli._parseVolumeStatusAll(tree) + self.assertEquals(status, + {'12345473-1237-1230-a877-831bc6e9dac2': + {'bricks': ['192.168.122.167:/tmp/V3-b1', + '192.168.122.168:/tmp/V3-b1'], + 'taskType': 'remove-brick', + 'volumeId': + '03eace73-1237-1230-a877-831bc6e9dac2', + 'volumeName': 'V3'}, + '12345473-1237-49d0-a877-831bc6e9dac2': + {'bricks': ['192.168.122.167:/tmp/V2-b1', + '192.168.122.168:/tmp/V2-b1'], + 'taskType': 'replace-brick', + 'volumeId': + '03eace73-1237-49d0-a877-831bc6e9dac2', + 'volumeName': 'V2'}, + '12345473-9197-49d0-a877-831bc6e9dac2': + {'bricks': [], + 'taskType': 'rebalance', + 'volumeId': + '03eace73-9197-49d0-a877-831bc6e9dac2', + 'volumeName': 'V1'}}) + + def test_parseVolumeStatusAll(self): + self._parseVolumeStatusAll_test() diff --git a/vdsm/gluster/cli.py b/vdsm/gluster/cli.py index 95de106..1f464f6 100644 --- a/vdsm/gluster/cli.py +++ b/vdsm/gluster/cli.py @@ -84,6 +84,55 @@ raise ge.GlusterCmdFailedException(rc=rv, err=[msg]) +class TaskType: + REBALANCE = 'rebalance' + REPLACE_BRICK = 'replace-brick' + REMOVE_BRICK = 'remove-brick' + + +def _parseVolumeStatusAll(tree): + """ + returns {TaskId: {'volumeName': VolumeName, + 'volumeId': VolumeId, + 'taskType': TaskType, + 'bricks': BrickList}, ...} + """ + tasks = {} + for el in tree.findall('volumes/volume'): + volumeName = el.find('name').text + volumeId = el.find('id').text + for c in el.findall('tasks/task'): + taskType = c.find('name').text + taskId = c.find('id').text + bricks = [] + if taskType == TaskType.REPLACE_BRICK: + bricks.append(c.find('sourceBrick').text) + bricks.append(c.find('destBrick').text) + elif taskType == TaskType.REMOVE_BRICK: + for b in c.findall('brick'): + bricks.append(b.text) + elif taskType == TaskType.REBALANCE: + pass + tasks[taskId] = {'volumeName': volumeName, + 'volumeId': volumeId, + 'taskType': taskType, + 'bricks': bricks} + return tasks + + +@exportToSuperVdsm +def volumeStatusAll(): + command = _getGlusterVolCmd() + ["status", "all"] + try: + xmltree, out = _execGlusterXml(command) + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeStatusAllFailedException(rc=e.rc, err=e.err) + try: + return _parseVolumeStatusAll(xmltree) + except: + raise ge.GlusterXmlErrorException(err=out) + + def _parseVolumeInfo(out): if not out[0].strip(): del out[0] @@ -300,11 +349,15 @@ command.append("start") if force: command.append("force") - rc, out, err = _execGluster(command) - if rc: - raise ge.GlusterVolumeRebalanceStartFailedException(rc, out, err) - else: - return True + try: + xmltree, out = _execGlusterXml(command) + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeRebalanceStartFailedException(rc=e.rc, + err=e.err) + try: + return {'taskId': xmltree.find('id').text} + except: + raise ge.GlusterXmlErrorException(err=out) @exportToSuperVdsm @@ -312,84 +365,147 @@ command = _getGlusterVolCmd() + ["rebalance", volumeName, "stop"] if force: command.append('force') - rc, out, err = _execGluster(command) - if rc: - raise ge.GlusterVolumeRebalanceStopFailedException(rc, out, err) - else: + try: + _execGlusterXml(command) return True + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeRebalanceStopFailedException(rc=e.rc, + err=e.err) + + +class TaskStatus(): + RUNNING = 'RUNNING' + FAILED = 'FAILED' + COMPLETED = 'COMPLETED' + + +def _parseVolumeRebalanceRemoveBrickStatus(xmltree, mode): + """ + returns {'taskId': UUID, + 'host': [{'name': NAME, + 'id': HOSTID, + 'filesScanned': INT, + 'filesMoved': INT, + 'filesFailed': INT, + 'totalSizeMoved': INT, + 'status': TaskStatus},...] + 'summary': {'filesScanned': INT, + 'filesMoved': INT, + 'filesFailed': INT, + 'totalSizeMoved': INT, + 'status': TaskStatus}} + """ + if mode == 'rebalance': + tree = xmltree.find('volRebalance') + elif mode == 'remove-brick': + tree = xmltree.find('volRemoveBrick') + else: + return + status = \ + {'taskId': tree.find('id').text, + 'summary': \ + {'filesScanned': int(tree.find('summary/filesScanned').text), + 'filesMoved': int(tree.find('summary/filesMoved').text), + 'filesFailed': int(tree.find('summary/filesFailed').text), + 'totalSizeMoved': int(tree.find('summary/totalSizeMoved').text), + 'status': tree.find('summary/status').text}, + 'host': []} + for el in tree.findall('node'): + status['host'].append({'name': el.find('name').text, + 'id': el.find('id').text, + 'filesScanned': + int(el.find('filesScanned').text), + 'filesMoved': int(el.find('filesMoved').text), + 'filesFailed': int(el.find('filesFailed').text), + 'totalSizeMoved': + int(el.find('totalSizeMoved').text), + 'status': el.find('status').text}) + return status + + +def _parseVolumeRebalanceStatus(tree): + return _parseVolumeRebalanceRemoveBrickStatus(tree, 'rebalance') @exportToSuperVdsm def volumeRebalanceStatus(volumeName): - rc, out, err = _execGluster(_getGlusterVolCmd() + ["rebalance", volumeName, - "status"]) - if rc: - raise ge.GlusterVolumeRebalanceStatusFailedException(rc, out, err) - if 'in progress' in out[0]: - return BrickStatus.RUNNING, "\n".join(out) - elif 'complete' in out[0]: - return BrickStatus.COMPLETED, "\n".join(out) - else: - return BrickStatus.UNKNOWN, "\n".join(out) + command = _getGlusterVolCmd() + ["rebalance", volumeName, "status"] + try: + xmltree, out = _execGlusterXml(command) + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeRebalanceStatusFailedException(rc=e.rc, + err=e.err) + try: + return _parseVolumeRebalanceStatus(xmltree) + except: + raise ge.GlusterXmlErrorException(err=out) @exportToSuperVdsm def volumeReplaceBrickStart(volumeName, existingBrick, newBrick): - rc, out, err = _execGluster(_getGlusterVolCmd() + ["replace-brick", - volumeName, - existingBrick, newBrick, - "start"]) - if rc: - raise ge.GlusterVolumeReplaceBrickStartFailedException(rc, out, err) - else: - return True + command = _getGlusterVolCmd() + ["replace-brick", volumeName, + existingBrick, newBrick, "start"] + try: + xmltree, out = _execGlusterXml(command) + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeReplaceBrickStartFailedException(rc=e.rc, + err=e.err) + try: + return {'taskId': xmltree.find('id').text} + except: + raise ge.GlusterXmlErrorException(err=out) @exportToSuperVdsm def volumeReplaceBrickAbort(volumeName, existingBrick, newBrick): - rc, out, err = _execGluster(_getGlusterVolCmd() + ["replace-brick", - volumeName, - existingBrick, newBrick, - "abort"]) - if rc: - raise ge.GlusterVolumeReplaceBrickAbortFailedException(rc, out, err) - else: + command = _getGlusterVolCmd() + ["replace-brick", volumeName, + existingBrick, newBrick, "abort"] + try: + _execGlusterXml(command) return True + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeReplaceBrickAbortFailedException(rc=e.rc, + err=e.err) @exportToSuperVdsm def volumeReplaceBrickPause(volumeName, existingBrick, newBrick): - rc, out, err = _execGluster(_getGlusterVolCmd() + ["replace-brick", - volumeName, - existingBrick, newBrick, - "pause"]) - if rc: - raise ge.GlusterVolumeReplaceBrickPauseFailedException(rc, out, err) - else: + command = _getGlusterVolCmd() + ["replace-brick", volumeName, + existingBrick, newBrick, "pause"] + try: + _execGlusterXml(command) return True + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeReplaceBrickPauseFailedException(rc=e.rc, + err=e.err) + + +def _parseVolumeReplaceBrickStatus(tree): + """ + returns {'taskId': UUID, + 'filesMoved': INT, + 'movingFile': STRING, + 'status': TaskStatus}} + """ + return {'taskId': tree.find('volReplaceBrick/id').text, + 'filesMoved': int(tree.find('volReplaceBrick/filesMoved').text), + 'movingFile': tree.find('volReplaceBrick/movingFile').text, + 'status': tree.find('volReplaceBrick/status').text} @exportToSuperVdsm def volumeReplaceBrickStatus(volumeName, existingBrick, newBrick): - rc, out, err = _execGluster(_getGlusterVolCmd() + ["replace-brick", - volumeName, - existingBrick, newBrick, - "status"]) - if rc: - raise ge.GlusterVolumeReplaceBrickStatusFailedException(rc, out, - err) - message = "\n".join(out) - statLine = out[0].strip().upper() - if BrickStatus.PAUSED in statLine: - return BrickStatus.PAUSED, message - elif statLine.endswith('MIGRATION COMPLETE'): - return BrickStatus.COMPLETED, message - elif statLine.startswith('NUMBER OF FILES MIGRATED'): - return BrickStatus.RUNNING, message - elif statLine.endswith("UNKNOWN"): - return BrickStatus.UNKNOWN, message - else: - return BrickStatus.NA, message + command = _getGlusterVolCmd() + ["replace-brick", volumeName, + existingBrick, newBrick, "status"] + try: + xmltree, out = _execGlusterXml(command) + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeReplaceBrickStatusFailedException(rc=e.rc, + err=e.err) + try: + return _parseVolumeReplaceBrickStatus(xmltree) + except: + raise ge.GlusterXmlErrorException(err=out) @exportToSuperVdsm @@ -399,12 +515,12 @@ existingBrick, newBrick, "commit"] if force: command.append('force') - rc, out, err = _execGluster(command) - if rc: - raise ge.GlusterVolumeReplaceBrickCommitFailedException(rc, out, - err) - else: + try: + _execGlusterXml(command) return True + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeReplaceBrickCommitFailedException(rc=e.rc, + err=e.err) @exportToSuperVdsm @@ -413,12 +529,15 @@ if replicaCount: command += ["replica", "%s" % replicaCount] command += brickList + ["start"] - - rc, out, err = _execGluster(command) - if rc: - raise ge.GlusterVolumeRemoveBrickStartFailedException(rc, out, err) - else: - return True + try: + xmltree, out = _execGlusterXml(command) + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeRemoveBrickStartFailedException(rc=e.rc, + err=e.err) + try: + return {'taskId': xmltree.find('id').text} + except: + raise ge.GlusterXmlErrorException(err=out) @exportToSuperVdsm @@ -427,12 +546,16 @@ if replicaCount: command += ["replica", "%s" % replicaCount] command += brickList + ["stop"] - rc, out, err = _execGluster(command) - - if rc: - raise ge.GlusterVolumeRemoveBrickStopFailedException(rc, out, err) - else: + try: + _execGlusterXml(command) return True + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeRemoveBrickStopFailedException(rc=e.rc, + err=e.err) + + +def _parseVolumeRemoveBrickStatus(tree): + return _parseVolumeRebalanceRemoveBrickStatus(tree, 'remove-brick') @exportToSuperVdsm @@ -441,12 +564,15 @@ if replicaCount: command += ["replica", "%s" % replicaCount] command += brickList + ["status"] - rc, out, err = _execGluster(command) - - if rc: - raise ge.GlusterVolumeRemoveBrickStatusFailedException(rc, out, err) - else: - return "\n".join(out) + try: + xmltree, out = _execGlusterXml(command) + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeRemoveBrickStatusFailedException(rc=e.rc, + err=e.err) + try: + return _parseVolumeRemoveBrickStatus(xmltree) + except: + raise ge.GlusterXmlErrorException(err=out) @exportToSuperVdsm @@ -455,12 +581,12 @@ if replicaCount: command += ["replica", "%s" % replicaCount] command += brickList + ["commit"] - rc, out, err = _execGluster(command) - - if rc: - raise ge.GlusterVolumeRemoveBrickCommitFailedException(rc, out, err) - else: + try: + _execGlusterXml(command) return True + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeRemoveBrickCommitFailedException(rc=e.rc, + err=e.err) @exportToSuperVdsm @@ -469,12 +595,12 @@ if replicaCount: command += ["replica", "%s" % replicaCount] command += brickList + ["force"] - rc, out, err = _execGluster(command) - - if rc: - raise ge.GlusterVolumeRemoveBrickForceFailedException(rc, out, err) - else: + try: + _execGlusterXml(command) return True + except ge.GlusterCmdFailedException, e: + raise ge.GlusterVolumeRemoveBrickForceFailedException(rc=e.rc, + err=e.err) @exportToSuperVdsm diff --git a/vdsm/gluster/exception.py b/vdsm/gluster/exception.py index f4f497b..f209885 100644 --- a/vdsm/gluster/exception.py +++ b/vdsm/gluster/exception.py @@ -323,6 +323,11 @@ message = "Volume remove brick force failed" +class GlusterVolumeStatusAllFailedException(GlusterVolumeException): + code = 4158 + message = "Volume status all failed" + + # Host class GlusterHostException(GlusterException): code = 4400 diff --git a/vdsm/storage/taskManager.py b/vdsm/storage/taskManager.py index 3bc12f3..0a269cd 100644 --- a/vdsm/storage/taskManager.py +++ b/vdsm/storage/taskManager.py @@ -25,6 +25,12 @@ import storage_exception as se from task import Task, Job, TaskCleanType from threadPool import ThreadPool +try: + from gluster import cli as gcli + from gluster import exception as ge + _glusterEnabled = True +except ImportError: + _glusterEnabled = False class TaskManager: @@ -113,19 +119,82 @@ self.log.debug("Return: %s", subRes) return subRes - def getAllTasks(self): + def _getAllGlusterTasks(self): """ - Return Tasks for all public tasks. + Return all gluster tasks + """ + subRes = {} + if not _glusterEnabled: + return subRes + + for taskId, value in gcli.volumeStatusAll(): + msg = '' + state = '' + try: + if value['taskType'] == gcli.TaskType.REBALANCE: + status = gcli.volumeRebalanceStatus(value['volumeName']) + msg = ('Files [scanned: %d, moved: %d, failed: %d], ' + 'Total size moved: %d') % \ + (status['summary']['filesScanned'], + status['summary']['filesMoved'], + status['summary']['filesFailed'], + status['summary']['totalSizeMoved']) + state = status['summary']['status'] + elif value['taskType'] == gcli.TaskType.REMOVE_BRICK: + status = gcli.volumeRemoveBrickStatus(value['volumeName'], + value['bricks']) + msg = ('Files [scanned: %d, moved: %d, failed: %d], ' + 'Total size moved: %d') % \ + (status['summary']['filesScanned'], + status['summary']['filesMoved'], + status['summary']['filesFailed'], + status['summary']['totalSizeMoved']) + state = status['summary']['status'] + elif value['taskType'] == gcli.TaskType.REPLACE_BRICK: + status = gcli.volumeReplaceBrickStatus(value['volumeName'], + value['bricks'][0], + value['bricks'][1]) + msg = 'Files moved: %d, Moving file: %s' % \ + (status['filesMoved'], status['movingFile']) + state = status['status'] + except ge.GlusterException: + self.log.error("gluster exception occured", exc_info=True) + + subRes[taskId] = {"id": taskId, + "verb": value['volumeName'], + "state": state, + "code": value['taskType'], + "message": msg, + "result": '', + "tag": 'gluster'} + return subRes + + def getAllTasks(self, tag=[]): + """ + Return Tasks for all public tasks if param tag is empty, + else return tasks those tags are in param tag. """ self.log.debug("Entry.") subRes = {} for taskID, task in self._tasks.items(): try: - subRes[taskID] = task.getDetails() + if not tag: + subRes[taskID] = task.getDetails() + elif task.getTags() in tag: + subRes[taskID] = task.getDetails() except se.UnknownTask: # Return info for existing tasks only. self.log.warn("Unknown task %s. Maybe task was already " "cleared.", taskID) + + try: + if not tag: + subRes.update(self._getAllGlusterTasks()) + elif 'gluster' in tag: + subRes.update(self._getAllGlusterTasks()) + except ge.GlusterException: + self.log.error("gluster exception occured", exc_info=True) + self.log.debug("Return: %s", subRes) return subRes -- To view, visit http://gerrit.ovirt.org/7579 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9c765cbfebb5ba22f0d21efa04c824ea4daf6432 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Bala.FA <barumuga(a)redhat.com> Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com> Gerrit-Reviewer: Timothy Asir <tjeyasin(a)redhat.com>

5 16

Change in vdsm[master]: vm: Modify and save state after hotunplugging the disk
by xfrancis＠redhat.com 11 Oct '15

11 Oct '15

Xavi Francisco has uploaded a new change for review. Change subject: vm: Modify and save state after hotunplugging the disk ...................................................................... vm: Modify and save state after hotunplugging the disk The rationale behind this patch is to modify the way the hotunplug process is executed. Now when the hotunplug command is called we first remove the disk from the internal state and after that the libvirt hotunplug command is executed.The problem with that approach is that if VDSM restarts between the modification of the internal state and the actual unplugging of the disk, the disk never gets unplugged leaving the system in an inconsistent state. This patch solves the issue by executing the modification of the internal state after the actual unplugging has successfully happened. This state is later saved so if either the disk has been unplugged or not VDSM is able to recover its state in case of a restart. Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5 Signed-off-by: Xavi Francisco <xfrancis(a)redhat.com> --- M vdsm/virt/vm.py 1 file changed, 12 insertions(+), 20 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/87/28187/1 diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index bd670b9..1eb2adc 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -3449,20 +3449,6 @@ driveXml = drive.getXML().toprettyxml(encoding='utf-8') self.log.debug("Hotunplug disk xml: %s", driveXml) # Remove found disk from vm's drives list - if isVdsmImage(drive): - self.sdIds.remove(drive.domainID) - self._devices[DISK_DEVICES].remove(drive) - # Find and remove disk device from vm's conf - diskDev = None - for dev in self.conf['devices'][:]: - if (dev['type'] == DISK_DEVICES and - dev['path'] == drive.path): - with self._confLock: - self.conf['devices'].remove(dev) - diskDev = dev - break - - self.saveState() hooks.before_disk_hotunplug(driveXml, self.conf, params=drive.custom) @@ -3472,19 +3458,25 @@ self.log.error("Hotunplug failed", exc_info=True) if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: return errCode['noVM'] - self._devices[DISK_DEVICES].append(drive) - # Restore disk device in vm's conf and _devices - if diskDev: - with self._confLock: - self.conf['devices'].append(diskDev) - self.saveState() return { 'status': {'code': errCode['hotunplugDisk']['status']['code'], 'message': e.message}} else: + if isVdsmImage(drive): + self.sdIds.remove(drive.domainID) + self._devices[DISK_DEVICES].remove(drive) + # Find and remove disk device from vm's conf + for dev in self.conf['devices'][:]: + if (dev['type'] == DISK_DEVICES and + dev['path'] == drive.path): + with self._confLock: + self.conf['devices'].remove(dev) + break hooks.after_disk_hotunplug(driveXml, self.conf, params=drive.custom) self._cleanupDrives(drive) + finally: + self.saveState() return {'status': doneCode, 'vmList': self.status()} -- To view, visit http://gerrit.ovirt.org/28187 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5169fa16591283de33aa82c5730626bfd5d3eaf5 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Xavi Francisco <xfrancis(a)redhat.com>

6 6

Change in vdsm[master]: tests: use 'localhost' explicitly in test
by ykleinbe＠redhat.com 08 Oct '15

08 Oct '15

Yoav Kleinberger has uploaded a new change for review. Change subject: tests: use 'localhost' explicitly in test ...................................................................... tests: use 'localhost' explicitly in test Previously tests could on some machines (in case the machine has a non-default hostname). Now, since we use 'localhost' explicitly, this will not happen. Change-Id: I89990cff46e64120262e250eee9238b49c4edee4 Signed-off-by: Yoav Kleinberger <ykleinbe(a)redhat.com> --- M tests/functional/storageTests.py 1 file changed, 2 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/07/28107/1 diff --git a/tests/functional/storageTests.py b/tests/functional/storageTests.py index 80ba312..76ca91d 100644 --- a/tests/functional/storageTests.py +++ b/tests/functional/storageTests.py @@ -79,7 +79,8 @@ isSSL = config.getboolean('vars', 'ssl') if isSSL and os.geteuid() != 0: raise SkipTest("Must be root to use SSL connection to server") - self.s = vdscli.connect(useSSL=isSSL) + address = 'localhost:%s' % config.get('addresses', 'management_port') + self.s = vdscli.connect(hostPort=address, useSSL=isSSL) def assertVdsOK(self, vdsResult): # code == 0 means OK -- To view, visit http://gerrit.ovirt.org/28107 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I89990cff46e64120262e250eee9238b49c4edee4 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yoav Kleinberger <ykleinbe(a)redhat.com>

8 20

Change in vdsm[master]: multipath: Move all calls to multipath exe to a single method
by smizrahi＠redhat.com 10 Sep '15

10 Sep '15

Saggi Mizrahi has uploaded a new change for review. Change subject: multipath: Move all calls to multipath exe to a single method ...................................................................... multipath: Move all calls to multipath exe to a single method This makes the code a bit cleaner Change-Id: I52afc07a07a925ed7572eb369deb7c203edb04cd Signed-off-by: Saggi Mizrahi <smizrahi(a)redhat.com> --- M vdsm/storage/multipath.py 1 file changed, 11 insertions(+), 4 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/55/19255/1 diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py index 924d747..c31b5c3 100644 --- a/vdsm/storage/multipath.py +++ b/vdsm/storage/multipath.py @@ -94,6 +94,10 @@ ) +def _runCmd(args): + return misc.execCmd([constants.EXT_MULTIPATH] + args, sudo=True) + + def rescan(): """ Forces multipath daemon to rescan the list of available devices and @@ -108,8 +112,8 @@ supervdsm.getProxy().forceScsiScan() # Now let multipath daemon pick up new devices - cmd = [constants.EXT_MULTIPATH, "-r"] - misc.execCmd(cmd, sudo=True) + + _runCmd("-r") def isEnabled(): @@ -154,6 +158,10 @@ return False +def flushAll(): + _runCmd("-F") + + def setupMultipath(): """ Set up the multipath daemon configuration to the known and @@ -173,8 +181,7 @@ raise se.MultipathSetupError() misc.persistFile(MPATH_CONF) - # Flush all unused multipath device maps - misc.execCmd([constants.EXT_MULTIPATH, "-F"], sudo=True) + flushAll() cmd = [constants.EXT_VDSM_TOOL, "service-reload", "multipathd"] rc = misc.execCmd(cmd, sudo=True)[0] -- To view, visit http://gerrit.ovirt.org/19255 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I52afc07a07a925ed7572eb369deb7c203edb04cd Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>

10 25

Change in vdsm[master]: [WIP]Java Bindings: Proton support in Java Bindings
by smizrahi＠redhat.com 07 Sep '15

07 Sep '15

Saggi Mizrahi has uploaded a new change for review. Change subject: [WIP]Java Bindings: Proton support in Java Bindings ...................................................................... [WIP]Java Bindings: Proton support in Java Bindings Change-Id: I94c52e118cb63d7df84b89a9b93da7b9e477be91 Signed-off-by: Saggi Mizrahi <smizrahi(a)redhat.com> --- A client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonAuthenticator.java A client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonClient.java A client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonListener.java A client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonReactor.java A client/java/vdsm-json-rpc/src/test/java/org/ovirt/vdsm/jsonrpc/AmqpReactorTestHelper.java A client/java/vdsm-json-rpc/src/test/java/org/ovirt/vdsm/jsonrpc/TestJsonRpcClientAMQP.java 6 files changed, 844 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/15428/1 diff --git a/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonAuthenticator.java b/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonAuthenticator.java new file mode 100644 index 0000000..35c9099 --- /dev/null +++ b/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonAuthenticator.java @@ -0,0 +1,98 @@ +package org.ovirt.vdsm.reactors; + +import java.io.IOException; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.apache.qpid.proton.driver.Connector; +import org.apache.qpid.proton.engine.Sasl; +import org.apache.qpid.proton.engine.Sasl.SaslOutcome; +import org.apache.qpid.proton.engine.Sasl.SaslState; +import org.ovirt.vdsm.reactors.ProtonAuthenticator.AuthenticatorType; + +public final class ProtonAuthenticator { + + public enum AuthenticatorType { + + SERVER, CLIENT + } + + public enum ConnectionState { + + AUTHENTICATING, CONNECTED, FAILED + } + private ConnectionState _state; + final private AuthenticatorType _authType; + final private Connector<?> _connector; + + public ProtonAuthenticator(Connector<?> connector, + AuthenticatorType authType) { + _authType = authType; + setState(ConnectionState.AUTHENTICATING); + _connector = connector; + final Sasl sasl = _connector.sasl(); + if (authType == AuthenticatorType.CLIENT) { + sasl.setMechanisms(new String[]{"ANONYMOUS"}); + sasl.client(); + } + } + + private void setState(ConnectionState state) { + _state = state; + } + + public ConnectionState getState() { + return _state; + } + + public void authenticate() { + final Sasl sasl = _connector.sasl(); + + while (true) { + try { + this._connector.process(); + } catch (IOException ex) { + return; + } + final SaslState state = sasl.getState(); + switch (state) { + case PN_SASL_CONF: + if (_authType == AuthenticatorType.SERVER) { + sasl.setMechanisms(new String[]{"ANONYMOUS"}); + sasl.server(); + } + break; + case PN_SASL_STEP: + if (_authType == AuthenticatorType.SERVER) { + final String[] mechs = sasl.getRemoteMechanisms(); + if (mechs.length < 1) { + sasl.done(SaslOutcome.PN_SASL_AUTH); + break; + } + + final String mech = mechs[0]; + if (mech.equals("ANONYMOUS")) { + sasl.done(SaslOutcome.PN_SASL_OK); + } else { + sasl.done(SaslOutcome.PN_SASL_AUTH); + } + } + return; + case PN_SASL_PASS: + this.setState(ConnectionState.CONNECTED); + return; + case PN_SASL_FAIL: + this.setState(ConnectionState.FAILED); + return; + case PN_SASL_IDLE: + + break; + default: + return; + } + } + } + + public AuthenticatorType getAuthType() { + return _authType; + } +} diff --git a/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonClient.java b/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonClient.java new file mode 100644 index 0000000..4baffbf --- /dev/null +++ b/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonClient.java @@ -0,0 +1,224 @@ +package org.ovirt.vdsm.reactors; + +import java.nio.ByteBuffer; +import java.util.Calendar; +import java.util.UUID; +import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.Future; + +import javax.swing.event.EventListenerList; + +import org.apache.qpid.proton.amqp.Binary; +import org.apache.qpid.proton.amqp.messaging.Data; +import org.apache.qpid.proton.amqp.messaging.Section; +import org.apache.qpid.proton.engine.Delivery; +import org.apache.qpid.proton.engine.EndpointState; +import org.apache.qpid.proton.engine.Link; +import org.apache.qpid.proton.engine.Receiver; +import org.apache.qpid.proton.engine.Sender; +import org.apache.qpid.proton.engine.Session; +import org.apache.qpid.proton.message.Message; +import org.apache.qpid.proton.message.MessageFactory; +import org.apache.qpid.proton.message.impl.MessageFactoryImpl; + +public final class ProtonClient implements ReactorClient { + private final ProtonReactor _reactor; + final private Session _ssn; + + private Sender _sender; + private Receiver _receiver; + + private final int _CREDIT = 10; + private final ConcurrentLinkedQueue<ByteBuffer> _outbox; + private final EventListenerList _eventListeners; + private final int _deliveryTimeoutSec; + private final MessageFactory _msgFactory; + + public ProtonClient(ProtonReactor reactor, Session session) { + _ssn = session; + _sender = null; + _receiver = null; + _outbox = new ConcurrentLinkedQueue<>(); + _eventListeners = new EventListenerList(); + _deliveryTimeoutSec = 60 * 3; + _reactor = reactor; + _msgFactory = new MessageFactoryImpl(); + } + + @Override + public void addEventListener(EventListener el) { + synchronized (_eventListeners) { + _eventListeners.add(EventListener.class, el); + } + } + + @Override + public void removeEventListener(EventListener el) { + synchronized (_eventListeners) { + _eventListeners.remove(EventListener.class, el); + } + } + + private void emitOnMessageReceived(ByteBuffer message) { + synchronized (_eventListeners) { + final Class<EventListener> cls = EventListener.class; + final EventListener[] els = _eventListeners.getListeners(cls); + for (EventListener el : els) { + el.onMessageReceived(this, message); + } + } + } + + @Override + public void sendMessage(ByteBuffer message) { + _outbox.add(message); + _reactor.wakeup(); + } + + public void addLink(Link link) { + assert (link.getSession().equals(_ssn)); + + if (link instanceof Sender) { + if (_sender != null) { + // already have a sender + link.close(); + return; + } + + _sender = (Sender) link; + } else { + assert (link instanceof Receiver); + if (_receiver != null) { + // already have a receiver + link.close(); + return; + } + + _receiver = (Receiver) link; + _receiver.flow(_CREDIT); + } + link.open(); + } + + private Message _popOutgoingMessage() { + final ByteBuffer data = _outbox.poll(); + if (data == null) { + return null; + } + + final Section body = new Data(Binary.create(data)); + final Message msg = _msgFactory.createMessage(); + msg.setBody(body); + msg.setAddress(_sender.getTarget().toString()); + return msg; + } + + public void queueDeliveries() { + if (_sender == null) { + final String uuid = UUID.randomUUID().toString(); + _sender = _ssn.sender("Sender-" + uuid); + } + + while (_sender.getCredit() > 0) { + final Message m = _popOutgoingMessage(); + if (m == null) { + return; + } + + final String uuid = UUID.randomUUID().toString(); + final Delivery d = _sender + .delivery(("outgoing-" + uuid).getBytes()); + d.setContext(m); + } + } + + public void processDelivery(Delivery delivery) { + assert (_ssn.equals(delivery.getLink().getSession())); + + if (delivery.isReadable()) { + _processIncomingDelivery(delivery); + } else { + assert (delivery.isWritable()); + _processOutgoingDelivery(delivery); + } + } + + private void _processOutgoingDelivery(Delivery delivery) { + final Sender link = (Sender) delivery.getLink(); + assert (link.equals(_sender)); + + final Message msg = (Message) delivery.getContext(); + // TBD: Buffer can be reused forever. Change in case of + // performance issues. + ByteBuffer buff; + int i = 1; + int written = 0; + do { + buff = ByteBuffer.allocate(i * 4096); + written = msg.encode(buff.array(), 0, buff.capacity()); + i++; + } while (written == buff.capacity()); + + link.send(buff.array(), 0, written); + if (link.advance()) { + // Attach timeout to the delivery + final Calendar calendar = Calendar.getInstance(); + calendar.add(Calendar.SECOND, _deliveryTimeoutSec); + delivery.setContext(calendar); + } + } + + private void _processIncomingDelivery(Delivery delivery) { + int total = 0; + int read = 0; + ByteBuffer buff = ByteBuffer.allocate(4096); + + while (read >= 0) { + total += read; + if (total >= buff.capacity()) { + final ByteBuffer buff2 = ByteBuffer + .allocate(buff.capacity() * 2); + buff2.put(buff); + buff = buff2; + } + read = _receiver.recv(buff.array(), total, buff.capacity() - total); + } + + final Message msg = _msgFactory.createMessage(); + msg.decode(buff.array(), 0, total); + + assert (msg.getBody() instanceof Data); + final Data body = (Data) msg.getBody(); + final ByteBuffer bb = body.getValue().asByteBuffer(); + delivery.settle(); + emitOnMessageReceived(bb); + } + + @Override + public Future<Void> close() { + final Session ssn = _ssn; + return _reactor.queueOperation(new Callable<Void>() { + @Override + public Void call() { + ssn.close(); + return null; + } + }); + } + + @Override + public boolean closed() { + return _ssn.getLocalState().equals(EndpointState.CLOSED); + } + + public void removeLink(Link link) { + if (link.equals(_sender)) { + _sender = null; + } else { + assert (link.equals(_receiver)); + _receiver = null; + } + link.close(); + } +} diff --git a/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonListener.java b/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonListener.java new file mode 100644 index 0000000..35896f4 --- /dev/null +++ b/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonListener.java @@ -0,0 +1,42 @@ +package org.ovirt.vdsm.reactors; + +import java.io.IOException; +import java.util.concurrent.Callable; +import java.util.concurrent.Future; + +import org.apache.qpid.proton.driver.Listener; + +public final class ProtonListener implements ReactorListener { + private final EventListener _eventListener; + private Listener<ProtonListener> _listener; + private final ProtonReactor _reactor; + + public ProtonListener(ProtonReactor reactor, EventListener eventListener) { + _eventListener = eventListener; + _reactor = reactor; + } + + public void setListener(Listener<ProtonListener> l) { + _listener = l; + } + + public void accept(ReactorClient client) { + _eventListener.onAcccept(this, client); + } + + @Override + public Future<Void> close() { + final Listener<ProtonListener> l = _listener; + return _reactor.queueOperation(new Callable<Void>() { + @Override + public Void call() { + try { + l.close(); + } catch (IOException e) { + // already closed + } + return null; + } + }); + } +} diff --git a/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonReactor.java b/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonReactor.java new file mode 100644 index 0000000..b5a38b4 --- /dev/null +++ b/client/java/vdsm-json-rpc/src/main/java/org/ovirt/vdsm/reactors/ProtonReactor.java @@ -0,0 +1,452 @@ +package org.ovirt.vdsm.reactors; + +import java.io.IOException; +import java.util.Calendar; +import java.util.EnumSet; +import java.util.Iterator; +import java.util.concurrent.Callable; +import java.util.concurrent.Future; +import java.util.concurrent.FutureTask; + +import org.apache.qpid.proton.driver.Connector; +import org.apache.qpid.proton.driver.Driver; +import org.apache.qpid.proton.driver.Listener; +import org.apache.qpid.proton.driver.impl.DriverFactoryImpl; +import org.apache.qpid.proton.engine.Connection; +import org.apache.qpid.proton.engine.Delivery; +import org.apache.qpid.proton.engine.EndpointState; +import org.apache.qpid.proton.engine.EngineFactory; +import org.apache.qpid.proton.engine.Link; +import org.apache.qpid.proton.engine.Receiver; +import org.apache.qpid.proton.engine.Session; +import org.apache.qpid.proton.engine.impl.EngineFactoryImpl; +import org.ovirt.vdsm.reactors.ProtonAuthenticator.AuthenticatorType; +import org.ovirt.vdsm.reactors.ProtonAuthenticator.ConnectionState; +import org.ovirt.vdsm.util.ChainedOperation; +import org.ovirt.vdsm.util.ReactorScheduler; + +public final class ProtonReactor implements Reactor { + + private final Driver _driver; + private final ReactorScheduler _scheduler; + private boolean _isRunning; + final Object _syncRoot = new Object(); + final ProtonReactor reactor = this; + private EngineFactory _engineFactory; + + public boolean isRunning() { + return _isRunning; + } + + public ProtonReactor() throws IOException { + _engineFactory = new EngineFactoryImpl(); + _driver = new DriverFactoryImpl().createDriver(); + _isRunning = false; + _scheduler = new ReactorScheduler(); + } + + @Override + public void finalize() throws Throwable { + try { + _driver.destroy(); + } finally { + super.finalize(); + } + } + + // Creates a listener, returns null if failed to bind or reactor is not + // running; + @Override + public Future<ReactorListener> createListener(final String host, + final int port, + final ReactorListener.EventListener eventListener) { + + return queueOperation(new Callable<ReactorListener>() { + @Override + public ProtonListener call() { + + final ProtonListener listener = new ProtonListener(reactor, eventListener); + final Listener<ProtonListener> l = _driver.createListener(host, + port, listener); + + listener.setListener(l); + + if (l == null) { + return null; + } + + return listener; + } + }); + } + + @Override + public Future<ReactorClient> createClient(final String host, final int port) { + final Driver driver = _driver; + final EngineFactory engineFactory = _engineFactory; + + return queueOperation(new ChainedOperation.Operation<ReactorClient>() { + final private int _INIT = 1; + final private int _AUTHENTICATE = 2; + final private int _DONE = 3; + private int _state; + final private Driver _driver; + final private ProtonReactor _reactor; + private Connector<ProtonAuthenticator> _connector; + private ProtonAuthenticator _auth; + private boolean _done; + private boolean _cancelled; + private ReactorClient _result; + private EngineFactory _engineFactory; + + { + _driver = driver; + _reactor = reactor; + _state = _INIT; + _done = false; + _cancelled = false; + _engineFactory = engineFactory; + } + + @Override + public void call(final boolean cancelled) { + switch (_state) { + case _INIT: + if (cancelled) { + _cancelled = true; + _done = true; + return; + } + + _connector = this._driver.createConnector(host, port, null); + + final Connection connection = engineFactory.createConnection(); + _connector.setConnection(connection); + _auth = new ProtonAuthenticator(_connector, + AuthenticatorType.CLIENT); + _connector.setContext(_auth); + connection.open(); + _state = _AUTHENTICATE; + case _AUTHENTICATE: + if (cancelled) { + _cancelled = true; + _close(); + return; + } + + switch (_auth.getState()) { + case AUTHENTICATING: + _auth.authenticate(); + try { + _connector.process(); + } catch (IOException e) { + // ignore + } + return; + case FAILED: + _close(); + return; + case CONNECTED: + // Success ! + break; + } + + Session ssn = _connector.getConnection().session(); + ssn.open(); + _result = new ProtonClient(_reactor, ssn); + ssn.setContext(_result); + _done = true; + _state = _DONE; + } + } + + private void _close() { + _connector.getConnection().close(); + _connector.close(); + _done = true; + _result = null; + } + + @Override + public boolean isDone() { + return _done; + } + + @Override + public boolean isCancelled() { + return _cancelled; + } + + @Override + public ReactorClient getResult() { + return _result; + } + }); + } + + // Queues operation to be run in the serving loop. + public <T> Future<T> queueOperation(Callable<T> cb) { + final FutureTask<T> task = new FutureTask<>(cb); + _queueFuture(task); + return task; + } + + public <T> Future<T> queueOperation(ChainedOperation.Operation<T> op) { + final ChainedOperation<T> task = new ChainedOperation<>(op); + _queueFuture(task); + return task; + } + + private void _queueFuture(Future<?> op) { + synchronized (_scheduler) { + _scheduler.queueFuture(op); + wakeup(); + } + } + + private void _waitEvents() { + _driver.doWait(0); + } + + public void wakeup() { + _driver.wakeup(); + } + + @Override + public void serve() { + synchronized (_syncRoot) { + _isRunning = true; + } + + while (_isRunning) { + //_waitEvents(); + synchronized (_scheduler) { + _scheduler.performPendingOperations(); + } + _acceptConnectionRequests(); + _processConnectors(); + } + } + + private void _processConnectors() { + for (Connector<?> connector = _driver.connector(); connector != null; connector = _driver + .connector()) { + if (connector.isClosed()) { + connector.destroy(); + continue; + } + + try { + connector.process(); + } catch (IOException e) { + continue; + } + + final Object ctx = connector.getContext(); + assert (ctx instanceof ProtonAuthenticator); + + if (ctx instanceof ProtonAuthenticator) { + final ProtonAuthenticator auth = (ProtonAuthenticator) ctx; + ConnectionState cs = auth.getState(); + if (cs.equals(ConnectionState.AUTHENTICATING)) { + auth.authenticate(); + cs = auth.getState(); + } + + if (cs.equals(ConnectionState.CONNECTED)) { + if (connector.getConnection() == null) { + connector.setConnection(_engineFactory.createConnection()); + } + _processConnector(connector); + } + } + + try { + connector.process(); + } catch (IOException e) { + continue; + } + } + } + + private void _processConnector(Connector<?> connector) { + _initConnection(connector); + _openPendingSessions(connector); + _openLinks(connector); + _queueOutgoingDeliveries(connector); + _processDeliveries(connector); + _cleanDeliveries(connector); + _cleanLinks(connector); + _cleanSessions(connector); + } + + private void _cleanSessions(Connector<?> connector) { + final Connection conn = connector.getConnection(); + final EnumSet<EndpointState> localState = EnumSet + .of(EndpointState.ACTIVE); + final EnumSet<EndpointState> remoteState = EnumSet + .of(EndpointState.CLOSED); + + for (Session ssn = conn.sessionHead(localState, remoteState); ssn != null; ssn + .next(localState, remoteState)) { + + ssn.close(); + } + } + + private void _cleanLinks(Connector<?> connector) { + final Connection conn = connector.getConnection(); + final EnumSet<EndpointState> localState = EnumSet + .of(EndpointState.ACTIVE); + final EnumSet<EndpointState> remoteState = EnumSet + .of(EndpointState.CLOSED); + + for (Link link = conn.linkHead(localState, remoteState); link != null; link = link + .next(localState, remoteState)) { + + final ProtonClient ssn = _getClient(link.getSession()); + ssn.removeLink(link); + } + } + + private void _cleanDeliveries(Connector<?> connector) { + final Connection conn = connector.getConnection(); + final EnumSet<EndpointState> localState = EnumSet + .of(EndpointState.ACTIVE); + final EnumSet<EndpointState> remoteState = EnumSet + .allOf(EndpointState.class); + for (Link link = conn.linkHead(localState, remoteState); link != null; link = link + .next(localState, remoteState)) { + + if (link instanceof Receiver) { + // We settle all incoming deliveries upon receive + continue; + } + + Delivery d; + final Calendar now = Calendar.getInstance(); + for (Iterator<Delivery> iter = link.unsettled(); iter.hasNext();) { + d = iter.next(); + Object ctx = d.getContext(); + if (!(ctx instanceof Calendar)) { + // Has not been sent yet + continue; + } + + final Calendar timeout = (Calendar) ctx; + boolean remoteClosed = link.getRemoteState().equals( + EndpointState.CLOSED); + boolean timedOut = now.after(timeout); + if (d.remotelySettled() || timedOut || remoteClosed) { + d.settle(); + d.free(); + } + } + + } + + } + + private void _processDeliveries(Connector<?> connector) { + final Connection conn = connector.getConnection(); + for (Delivery delivery = conn.getWorkHead(); delivery != null; delivery = delivery + .getWorkNext()) { + + final ProtonClient client = _getClient(delivery.getLink() + .getSession()); + client.processDelivery(delivery); + } + } + + private void _queueOutgoingDeliveries(Connector<?> connector) { + final Connection conn = connector.getConnection(); + + final EnumSet<EndpointState> localState = EnumSet + .of(EndpointState.ACTIVE); + final EnumSet<EndpointState> remoteState = EnumSet + .allOf(EndpointState.class); + + for (Session ssn = conn.sessionHead(localState, remoteState); ssn != null; ssn = ssn + .next(localState, remoteState)) { + + final ProtonClient client = _getClient(ssn); + client.queueDeliveries(); + } + } + + private void _openLinks(Connector<?> connector) { + final Connection conn = connector.getConnection(); + final EnumSet<EndpointState> localState = EnumSet + .of(EndpointState.UNINITIALIZED); + final EnumSet<EndpointState> remoteState = EnumSet + .allOf(EndpointState.class); + for (Link link = conn.linkHead(localState, remoteState); link != null; link = link + .next(localState, remoteState)) { + + // configure the link + link.setSource(link.getRemoteSource()); + link.setTarget(link.getRemoteTarget()); + + final ProtonClient client = _getClient(link.getSession()); + client.addLink(link); + } + } + + private ProtonClient _getClient(Session ssn) { + return (ProtonClient) ssn.getContext(); + } + + private void _openPendingSessions(Connector<?> connector) { + final Connection conn = connector.getConnection(); + final EnumSet<EndpointState> localState = EnumSet + .of(EndpointState.UNINITIALIZED); + final EnumSet<EndpointState> remoteState = EnumSet + .allOf(EndpointState.class); + + for (Session ssn = conn.sessionHead(localState, remoteState); ssn != null; ssn = ssn + .next(localState, remoteState)) { + + final ProtonClient client = new ProtonClient(this, ssn); + ssn.setContext(client); + final Object ctx = connector.getContext(); + assert (ctx instanceof ProtonAuthenticator); + ProtonAuthenticator auth = (ProtonAuthenticator) ctx; + if (auth.getAuthType() == AuthenticatorType.SERVER) { + ssn.open(); + final ProtonListener l = (ProtonListener) ctx; + l.accept(client); + } else { + ssn.close(); + } + } + } + + private void _initConnection(Connector<?> connector) { + final Connection conn = connector.getConnection(); + if (conn.getLocalState().equals(EndpointState.UNINITIALIZED)) { + conn.open(); + } + } + + private void _acceptConnectionRequests() { + for (final Listener<?> l : _driver.listeners()) { + + @SuppressWarnings("unchecked") + final Connector<ProtonAuthenticator> connector = (Connector<ProtonAuthenticator>) l + .accept(); + if (connector == null) { + continue; + } + connector.setContext(new ProtonAuthenticator(connector, + AuthenticatorType.SERVER)); + } + } + + public void stop() { + synchronized (_syncRoot) { + _isRunning = false; + } + + wakeup(); + } +} \ No newline at end of file diff --git a/client/java/vdsm-json-rpc/src/test/java/org/ovirt/vdsm/jsonrpc/AmqpReactorTestHelper.java b/client/java/vdsm-json-rpc/src/test/java/org/ovirt/vdsm/jsonrpc/AmqpReactorTestHelper.java new file mode 100644 index 0000000..46d9cc3 --- /dev/null +++ b/client/java/vdsm-json-rpc/src/test/java/org/ovirt/vdsm/jsonrpc/AmqpReactorTestHelper.java @@ -0,0 +1,18 @@ +package org.ovirt.vdsm.jsonrpc; + +import java.io.IOException; +import org.ovirt.vdsm.reactors.ProtonReactor; +import org.ovirt.vdsm.reactors.Reactor; + +public class AmqpReactorTestHelper implements ReactorTestHelper { + @Override + public Reactor createReactor() throws IOException { + return new ProtonReactor(); + } + + @Override + public String getUriScheme() { + return "amqp"; + } + +} diff --git a/client/java/vdsm-json-rpc/src/test/java/org/ovirt/vdsm/jsonrpc/TestJsonRpcClientAMQP.java b/client/java/vdsm-json-rpc/src/test/java/org/ovirt/vdsm/jsonrpc/TestJsonRpcClientAMQP.java new file mode 100644 index 0000000..9e0c24c --- /dev/null +++ b/client/java/vdsm-json-rpc/src/test/java/org/ovirt/vdsm/jsonrpc/TestJsonRpcClientAMQP.java @@ -0,0 +1,10 @@ +package org.ovirt.vdsm.jsonrpc; + +public class TestJsonRpcClientAMQP extends TestJsonRpcClient { + + @Override + protected ReactorTestHelper getHelper() { + return new AmqpReactorTestHelper(); + } + +} -- To view, visit http://gerrit.ovirt.org/15428 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I94c52e118cb63d7df84b89a9b93da7b9e477be91 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi <smizrahi(a)redhat.com>

5 5

Change in vdsm[master]: [WIP] vdsm: add support for PCI passthrough
by mpoledni＠redhat.com 01 Sep '15

01 Sep '15

Martin Polednik has uploaded a new change for review. Change subject: [WIP] vdsm: add support for PCI passthrough ...................................................................... [WIP] vdsm: add support for PCI passthrough required functionality: * report PCI devices available on host [x] * handle createVm xml generation [x] * hotplugHostdev [ ] (required for after-migration) * hotpunlugHostdev [ ] (required for migration) Change-Id: I363d2622d72ca2db75f60032fe0892c348bab121 Signed-off-by: Martin Polednik <mpoledni(a)redhat.com> --- M lib/vdsm/define.py M vdsm/caps.py M vdsm/vm.py 3 files changed, 83 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/62/22462/1 diff --git a/lib/vdsm/define.py b/lib/vdsm/define.py index eb78633..9605f93 100644 --- a/lib/vdsm/define.py +++ b/lib/vdsm/define.py @@ -132,6 +132,12 @@ 'transientErr': {'status': { 'code': 59, 'message': 'Action not permitted on a VM with transient disks'}}, + 'hotplugHostdev': {'status': { + 'code': 60, + 'message': 'Failed to hotplug hostdev'}}, + 'hotunplugHostdev': {'status': { + 'code': 61, + 'message': 'Failed to hotunplug hostdev'}}, 'recovery': {'status': { 'code': 99, 'message': 'Recovering from crash or Initializing'}}, diff --git a/vdsm/caps.py b/vdsm/caps.py index 3839134..d6af375 100644 --- a/vdsm/caps.py +++ b/vdsm/caps.py @@ -308,6 +308,38 @@ return dict(release=release, version=version, name=osname) +def hostdevList(): + devices = [] + for device in libvirtconnection.get().listAllDevices(): + devXML = minidom.parseString(device.XMLDesc()) + dev = {} + + # we have to grab attributes that will most likely not only + # uniquely identify device, but also serve as human readable + # representation of the device + try: + dev['name'] = devXML.getElementsByTagName('name')[0].\ + childNodes[0].data + capability = devXML.getElementsByTagName('capability')[0] + try: + dev['product'] = capability.getElementsByTagName('product')[0]\ + .childNodes[0].data + dev['vendor'] = capability.getElementsByTagName('vendor')[0].\ + childNodes[0].data + except IndexError: + # althought the retrieval of product/vendor was not successful, + # we can still report back the name + pass + except IndexError: + # should device not have a name, there is nothing engine could send + # back that we could use to uniquely identify and initiate a device + continue + + devices.append(dev) + + return devices + + def get(): targetArch = platform.machine() @@ -360,6 +392,7 @@ config.getint('vars', 'extra_mem_reserve')) caps['guestOverhead'] = config.get('vars', 'guest_ram_overhead') caps['rngSources'] = _getRngSources() + caps['hostDevices'] = hostdevList() return caps diff --git a/vdsm/vm.py b/vdsm/vm.py index a5d923b..a477bc9 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -78,6 +78,7 @@ WATCHDOG_DEVICES = 'watchdog' CONSOLE_DEVICES = 'console' SMARTCARD_DEVICES = 'smartcard' +HOSTDEV_DEVICES = 'hostdev' def isVdsmImage(drive): @@ -1656,6 +1657,27 @@ return m +class HostDevice(VmDevice): + def getXML(self): + """ + Create domxml for a hostdev device. + + <devices> + <hostdev mode='subsystem' type='usb'> + <source startupPolicy='optional'> + <vendor id='0x1234'/> + <product id='0xbeef'/> + </source> + <boot order='2'/> + </hostdev> + </devices> + """ + # libvirt gives us direct api call to construct the XML + return xml.dom.minidom.parseString(libvirtconnection.get(). + nodeDeviceLookupByName(self.name). + XMLDesc()) + + class WatchdogDevice(VmDevice): def __init__(self, *args, **kwargs): super(WatchdogDevice, self).__init__(*args, **kwargs) @@ -1769,7 +1791,8 @@ (CONSOLE_DEVICES, ConsoleDevice), (REDIR_DEVICES, RedirDevice), (RNG_DEVICES, RngDevice), - (SMARTCARD_DEVICES, SmartCardDevice)) + (SMARTCARD_DEVICES, SmartCardDevice), + (HOSTDEV_DEVICES, HostDevice)) def _makeDeviceDict(self): return dict((dev, []) for dev, _ in self.DeviceMapping) @@ -3127,6 +3150,26 @@ break + def hotplugHostdev(self, params): + hostdev = HostDevice(self.conf, self.log, **params) + self._devices[HOSTDEV_DEVICES].append(hostdev) + hostdevXML = hostdev.getXML().toprettyxml(encoding='utf-8') + hostdev._deviceXML = hostdevXML + self.log.debug("Hotplug hostdev xml: %s", hostdevXML) + + try: + self._dom.attachDevice(hostdevXML) + except libvirt.libvirtError as e: + self.log.error("Hotplug failed", exc_info=True) + if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN: + return errCode['noVM'] + return {'status': {'code': + errCode['hotplugHostdev']['status']['code'], + 'message': e.message}} + + def hotunplugHostdev(self, name): + pass + def hotplugNic(self, params): if self.isMigrating(): return errCode['migInProgress'] -- To view, visit http://gerrit.ovirt.org/22462 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I363d2622d72ca2db75f60032fe0892c348bab121 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpoledni(a)redhat.com>

8 50

Change in vdsm[master]: vm: Require format attribute for drives
by Nir Soffer 13 Aug '15

13 Aug '15

Nir Soffer has uploaded a new change for review. Change subject: vm: Require format attribute for drives ...................................................................... vm: Require format attribute for drives We have seen sampling errors where a drive has no "format" attribute. These errors spam the system logs, and does not help to debug the real issue - why the required format attribute is not set? This patch ensure that a drive cannot be created without a format attribute, avoding log spam by sampling errors, and hopefully revealing the real reason for this bug. One broken test creating a drive without a format was fixed and new test ensure that creating drive without a format raises. Change-Id: I01ab1e071ecb76f383cc6dc7d99782e10cc90136 Relates-To: http://gerrit.ovirt.org/22551 Relates-To: https://bugzilla.redhat.com/994534 Relates-To: http://lists.ovirt.org/pipermail/users/2014-February/021007.html Bug-Url: https://bugzilla.redhat.com/1055437 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M tests/vmTests.py M vdsm/vm.py 2 files changed, 28 insertions(+), 3 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/34/24234/1 diff --git a/tests/vmTests.py b/tests/vmTests.py index 1e0a3f6..724d458 100644 --- a/tests/vmTests.py +++ b/tests/vmTests.py @@ -449,6 +449,18 @@ redir = vm.RedirDevice(self.conf, self.log, **dev) self.assertXML(redir.getXML(), redirXML) + def testDriveRequiredParameters(self): + # TODO: It is not clear what are the other required parameters, and it + # the parameters depend on the type of drive. We probbaly need bigger + # test here that test many combinations. + # Currently this test only missing "format" attribute. + conf = {'index': '2', 'propagateErrors': 'off', 'iface': 'ide', + 'name': 'hdc', 'device': 'cdrom', 'path': '/tmp/fedora.iso', + 'type': 'disk', 'readonly': 'True', 'shared': 'none', + 'serial': '54-a672-23e5b495a9ea'} + self.assertRaises(vm.InvalidDeviceParameters, vm.Drive, {}, self.log, + **conf) + def testDriveSharedStatus(self): sharedConfigs = [ # Backward compatibility @@ -470,7 +482,8 @@ 'exclusive', 'shared', 'none', 'transient', ] - driveConfig = {'index': '0', 'iface': 'virtio', 'device': 'disk'} + driveConfig = {'index': '0', 'iface': 'virtio', 'device': 'disk', + 'format': 'raw'} for driveInput, driveOutput in zip(sharedConfigs, expectedStates): driveInput.update(driveConfig) diff --git a/vdsm/vm.py b/vdsm/vm.py index aae8bd6..b7151b1 100644 --- a/vdsm/vm.py +++ b/vdsm/vm.py @@ -80,6 +80,10 @@ SMARTCARD_DEVICES = 'smartcard' +class InvalidDeviceParameters(Exception): + """ Raised when creating device with invalid parameters """ + + def isVdsmImage(drive): """ Tell if drive looks like a vdsm image @@ -1438,6 +1442,9 @@ VOLWM_CHUNK_REPLICATE_MULT = 2 # Chunk multiplier during replication def __init__(self, conf, log, **kwargs): + if 'format' not in kwargs: + raise InvalidDeviceParameters('"format" attribute is required:' + ' %r' % kwargs) if not kwargs.get('serial'): self.serial = kwargs.get('imageID'[-20:]) or '' VmDevice.__init__(self, conf, log, **kwargs) @@ -3108,8 +3115,13 @@ for devType, devClass in self.DeviceMapping: for dev in devices[devType]: - self._devices[devType].append(devClass(self.conf, self.log, - **dev)) + try: + drive = devClass(self.conf, self.log, **dev) + except InvalidDeviceParameters as e: + self.log.error('Ignoring device with invalid parameters:' + ' %s', e, exc_info=True) + else: + self._devices[devType].append(drive) # We should set this event as a last part of drives initialization self._pathsPreparedEvent.set() -- To view, visit http://gerrit.ovirt.org/24234 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I01ab1e071ecb76f383cc6dc7d99782e10cc90136 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

6 12

Change in vdsm[master]: resourceManager: Keep resource state if registerResource fails
by Nir Soffer 13 Aug '15

13 Aug '15

Nir Soffer has uploaded a new change for review. Change subject: resourceManager: Keep resource state if registerResource fails ...................................................................... resourceManager: Keep resource state if registerResource fails Previous code was increasing resource activeUsers counter, but exceptions raised after that caused the method to fail, leaving a locked resources that nobody can release. Such locked resource may lead to failure of any pool operation, making the host non-operational, and requiring a restart of vdsm. The failure in the field was caused by Python logging bug, raising OSError when message was logged when log file was rotated. However, such failure can happen everywhere, and locking code must be written in such way that failure would never leave a resource locked. This patch ensure that resource is added and activeUsers counter is increased only if everything else was fine. Since simulating logging error is hard, the tests monkeypatch the RequestRef class to simulate a failure. This is little ugly, depending on internal implementation detail, but I could not find a cleaner way. Change-Id: I16abf41ebc8a8a99b292d38c945074752254a34b Relates-To: https://bugzilla.redhat.com/1065650 Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> --- M tests/resourceManagerTests.py M vdsm/storage/resourceManager.py 2 files changed, 50 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/84/25284/1 diff --git a/tests/resourceManagerTests.py b/tests/resourceManagerTests.py index 01b0669..e2b6461 100644 --- a/tests/resourceManagerTests.py +++ b/tests/resourceManagerTests.py @@ -29,6 +29,7 @@ import storage.resourceManager as resourceManager from testrunner import VdsmTestCase as TestCaseBase from testValidation import slowtest, stresstest +import monkeypatch class NullResourceFactory(resourceManager.SimpleResourceFactory): @@ -209,6 +210,32 @@ ex.__class__.__name__) self.fail("Managed to access an attribute not exposed by wrapper") + + def testRegisterResourceFailureExclusive(self): + # This regeisterion must fail + with monkeypatch.MonkeyPatchScope( + [(resourceManager, 'RequestRef', FakeRequestRef)]): + self.assertRaises(Failure, self.manager.registerResource, "string", + "test", resourceManager.LockType.exclusive, None) + + # And it should not leave a locked resource + with self.manager.acquireResource("string", "test", + resourceManager.LockType.exclusive, + 0): + pass + + def testRegisterResourceFailureShared(self): + # This regeisterion must fail + with monkeypatch.MonkeyPatchScope( + [(resourceManager, 'RequestRef', FakeRequestRef)]): + self.assertRaises(Failure, self.manager.registerResource, "string", + "test", resourceManager.LockType.shared, None) + + # And it should not leave a locked resource + with self.manager.acquireResource("string", "test", + resourceManager.LockType.exclusive, + 0): + pass def testAccessAttributeNotExposedByRequestRef(self): resources = [] @@ -705,3 +732,14 @@ except: resourceManager.ResourceManager._instance = None raise + +# Helpers + + +class Failure(Exception): + """ Unique exception for testing """ + + +def FakeRequestRef(*a, **kw): + """ Used to simulate failures when registering a resource """ + raise Failure() diff --git a/vdsm/storage/resourceManager.py b/vdsm/storage/resourceManager.py index 1be1450..ce144cf 100644 --- a/vdsm/storage/resourceManager.py +++ b/vdsm/storage/resourceManager.py @@ -559,23 +559,25 @@ if len(resource.queue) == 0 and \ resource.currentLock == LockType.shared and \ request.lockType == LockType.shared: - resource.activeUsers += 1 self._log.debug("Resource '%s' found in shared state " "and queue is empty, Joining current " "shared lock (%d active users)", - fullName, resource.activeUsers) + fullName, resource.activeUsers + 1) request.grant() + ref = RequestRef(request) contextCleanup.defer(request.emit, ResourceRef(namespace, name, resource.realObj, request.reqID)) - return RequestRef(request) + resource.activeUsers += 1 + return ref - resource.queue.insert(0, request) self._log.debug("Resource '%s' is currently locked, " "Entering queue (%d in queue)", - fullName, len(resource.queue)) - return RequestRef(request) + fullName, len(resource.queue) + 1) + ref = RequestRef(request) + resource.queue.insert(0, request) + return ref # TODO : Creating the object inside the namespace lock causes # the entire namespace to lock and might cause @@ -592,19 +594,20 @@ contextCleanup.defer(request.cancel) return RequestRef(request) - resource = resources[name] = ResourceManager.ResourceInfo( - obj, namespace, name) + resource = ResourceManager.ResourceInfo(obj, namespace, name) resource.currentLock = request.lockType resource.activeUsers += 1 self._log.debug("Resource '%s' is free. Now locking as '%s' " "(1 active user)", fullName, request.lockType) request.grant() + ref = RequestRef(request) contextCleanup.defer(request.emit, ResourceRef(namespace, name, resource.realObj, request.reqID)) - return RequestRef(request) + resources[name] = resource + return ref def releaseResource(self, namespace, name): # WARN : unlike in resource acquire the user now has the request -- To view, visit http://gerrit.ovirt.org/25284 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I16abf41ebc8a8a99b292d38c945074752254a34b Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <nsoffer(a)redhat.com>

8 31

Change in vdsm[master]: call stop_event_loop upon exit
by Dan Kenigsberg 04 Aug '15

04 Aug '15

Dan Kenigsberg has uploaded a new change for review. Change subject: call stop_event_loop upon exit ...................................................................... call stop_event_loop upon exit For cleanliness, whomever starts a thread should stop it when it is no longer needed. Change-Id: I9ab0d9b7be976e37a89a96d2f09a353186008731 Signed-off-by: Dan Kenigsberg <danken(a)redhat.com> --- M vdsm/vdsm 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/32/26532/1 diff --git a/vdsm/vdsm b/vdsm/vdsm index 652797c..fd9b3f8 100755 --- a/vdsm/vdsm +++ b/vdsm/vdsm @@ -81,6 +81,7 @@ signal.pause() finally: cif.prepareForShutdown() + libvirtconnection.stop_event_loop() def run(pidfile=None): -- To view, visit http://gerrit.ovirt.org/26532 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I9ab0d9b7be976e37a89a96d2f09a353186008731 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Dan Kenigsberg <danken(a)redhat.com>

7 19

Change in vdsm[master]: virt: graphdev: support headless VM
by fromani＠redhat.com 14 Jul '15

14 Jul '15

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> --- 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>

7 53

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches May 2014