Francesco Romani has uploaded a new change for review. Change subject: janitorial: add the response.success() helper ......................................................................
janitorial: add the response.success() helper This patch augments the response module with the success() helper, so VDSM code has now a nicer and cleaner alternative to the bare doneCode which was used before. response.success() also allows caller code to augment the response code with custom fields, as it is done many times in Vm class. Change-Id: I5fcd6c832f3a16a543357570c591c1f9a907c97a Signed-off-by: Francesco Romani <[email protected]> --- M lib/vdsm/response.py M vdsm/virt/vm.py 2 files changed, 41 insertions(+), 34 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/38270/1 diff --git a/lib/vdsm/response.py b/lib/vdsm/response.py index 7b0ba4e..aaac08e 100644 --- a/lib/vdsm/response.py +++ b/lib/vdsm/response.py @@ -19,7 +19,13 @@ # -from vdsm.define import errCode +from vdsm.define import doneCode, errCode + + +def success(**kwargs): + res = {'status': doneCode} + res.update(kwargs) + return res def error(key, message=None): diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py index 4be63ce..92cfac3 100644 --- a/vdsm/virt/vm.py +++ b/vdsm/virt/vm.py @@ -42,7 +42,7 @@ from vdsm import utils from vdsm.compat import pickle from vdsm.config import config -from vdsm.define import ERROR, NORMAL, doneCode +from vdsm.define import ERROR, NORMAL from vdsm.netinfo import DUMMY_BRIDGE from storage import outOfProcess as oop from storage import sd @@ -1600,7 +1600,7 @@ del self.conf['pauseCode'] except KeyError: pass - return {'status': doneCode, 'output': ['']} + return response.success(output=['']) finally: if not guestCpuLocked: self._guestCpuLock.release() @@ -1617,7 +1617,7 @@ guestCpuLocked=True) self._logGuestCpuStatus('pause') self._lastStatus = afterState - return {'status': doneCode, 'output': ['']} + return response.success(output=['']) finally: if not guestCpuLocked: self._guestCpuLock.release() @@ -2368,7 +2368,7 @@ self.hotunplugNic({'nic': nicParams}) return response.error('hotplugNic', e.message) - return {'status': doneCode, 'vmList': self.status()} + return response.success(vmList=self.status()) def _lookupDeviceByAlias(self, devType, alias): for dev in self._devices[devType][:]: @@ -2426,7 +2426,7 @@ with self.setLinkAndNetwork(netDev, netConf, linkValue, network, custom, specParams): with self.updatePortMirroring(netConf, netsToMirror): - return {'status': doneCode, 'vmList': self.status()} + return response.success(vmList=self.status()) except (LookupError, SetLinkAndNetworkError, UpdatePortMirroringError) as e: @@ -2597,7 +2597,7 @@ hooks.after_nic_hotunplug(nicXml, self.conf, params=nic.custom) - return {'status': doneCode, 'vmList': self.status()} + return response.success(vmList=self.status()) def setNumberOfCpus(self, numberOfCpus): @@ -2618,7 +2618,7 @@ self.conf['smp'] = str(numberOfCpus) self.saveState() hooks.after_set_num_of_cpus() - return {'status': doneCode, 'vmList': self.status()} + return response.success(vmList=self.status()) def updateVmPolicy(self, params): """ @@ -2714,7 +2714,7 @@ else: return response.error('updateVmPolicyErr', e.message) - return {'status': doneCode} + return response.success() def _getVmPolicy(self): """ @@ -2819,7 +2819,7 @@ found_device.name, xml) found_device._deviceXML = xml - return {'status': doneCode} + return response.success() def _createTransientDisk(self, diskParams): if (diskParams.get('shared', None) != @@ -2903,7 +2903,7 @@ hooks.after_disk_hotplug(driveXml, self.conf, params=drive.custom) - return {'status': doneCode, 'vmList': self.status()} + return response.success(vmList=self.status()) def hotunplugDisk(self, params): if self.isMigrating(): @@ -2960,7 +2960,7 @@ params=drive.custom) self._cleanupDrives(drive) - return {'status': doneCode, 'vmList': self.status()} + return response.success(vmList=self.status()) def _readPauseCode(self): state, reason = self._dom.state(0) @@ -3282,7 +3282,7 @@ # If all the drives are the current ones, return success if len(newDrives) == 0: self.log.debug('all the drives are already in use, success') - return {'status': doneCode} + return response.success() preparedDrives = {} @@ -3388,9 +3388,10 @@ # Returning quiesce to notify the manager whether the guest agent # froze and flushed the filesystems or not. - return {'status': doneCode, 'quiesce': - (snapFlags & libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE - == libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE)} + quiesceFlag = ( + snapFlags & libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE + == libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE) + return response.success(quiesce=quiesceFlag) def _setDiskReplica(self, srcDrive, dstDisk): """ @@ -3494,7 +3495,7 @@ self.log.exception("Initial extension request failed for %s", srcDrive.name) - return {'status': doneCode} + return response.success() def diskReplicateFinish(self, srcDisk, dstDisk): try: @@ -3578,7 +3579,7 @@ self._delDiskReplica(srcDrive) self.startDisksStatsCollection() - return {'status': doneCode} + return response.success() def _diskSizeExtendCow(self, drive, newSizeBytes): # Apparently this is what libvirt would do anyway, except that @@ -3615,7 +3616,7 @@ drive.domainID, drive.poolID, drive.imageID, drive.volumeID, sizeRoundedBytes) - return {'status': doneCode, 'size': str(sizeRoundedBytes)} + return response.success(size=str(sizeRoundedBytes)) def _diskSizeExtendRaw(self, drive, newSizeBytes): # Picking up the volume size extension @@ -3650,7 +3651,7 @@ "reboot", sizeRoundedBytes, exc_info=True) return response.error('updateDevice') - return {'status': doneCode, 'size': str(sizeRoundedBytes)} + return response.success(size=str(sizeRoundedBytes)) def diskSizeExtend(self, driveSpecs, newSizeBytes): try: @@ -3728,7 +3729,7 @@ self.cif.teardownVolumePath(self.conf[vmDev]) self.conf[vmDev] = path - return {'status': doneCode, 'vmList': self.status()} + return response.success(vmList=self.status()) def setTicket(self, otp, seconds, connAct, params): """ @@ -3754,7 +3755,7 @@ res = response.error('ticketErr', unicode(tmo)) else: hooks.after_vm_set_ticket(self._domain.xml, self.conf, params) - res = {'status': doneCode} + res = response.success() return res def _reviveTicket(self, newlife): @@ -3852,7 +3853,7 @@ with self._releaseLock: if self._released: - return {'status': doneCode} + return response.success() self.log.info('Release VM resources') self.lastStatus = vmstatus.POWERING_DOWN @@ -3862,9 +3863,9 @@ self._vmStats.stop() self.guestAgent.stop() if self._dom: - response = self._destroyVmGraceful() - if response['status']['code']: - return response + res = self._destroyVmGraceful() + if res['status']['code']: + return res # Wait for any Live Merge cleanup threads. This will only block in # the extremely rare case where a VM is being powered off at the @@ -3884,7 +3885,7 @@ self._released = True - return {'status': doneCode} + return response.success() def _destroyVmGraceful(self): try: @@ -3901,7 +3902,7 @@ if e.get_error_code() == libvirt.VIR_ERR_OPERATION_FAILED: return self._destroyVmForceful() return response.error('destroyErr') - return {'status': doneCode} + return response.success() def _destroyVmForceful(self): try: @@ -3910,7 +3911,7 @@ self.log.warning("Failed to destroy VM '%s'", self.conf['vmId'], exc_info=True) return response.error('destroyErr') - return {'status': doneCode} + return response.success() def deleteVm(self): """ @@ -3932,7 +3933,7 @@ # Clean VM from the system self.deleteVm() - return {'status': doneCode} + return response.success() def doDestroy(self): for dev in self._customDevices(): @@ -3972,7 +3973,7 @@ dev['target'] = target # persist the target value to make it consistent after recovery self.saveState() - return {'status': doneCode} + return response.success() def setCpuTuneQuota(self, quota): try: @@ -3982,7 +3983,7 @@ 'an integer is required for period') except libvirt.libvirtError as e: return self._reportException(key='cpuTuneErr', msg=e.message) - return {'status': doneCode} + return response.success() def setCpuTunePeriod(self, period): try: @@ -3992,7 +3993,7 @@ 'an integer is required for period') except libvirt.libvirtError as e: return self._reportException(key='cpuTuneErr', msg=e.message) - return {'status': doneCode} + return response.success() def _reportException(self, key, msg=None): """ @@ -4766,7 +4767,7 @@ # of getVmStats after this returns will see the new job self._vmStats.sampleVmJobs() - return {'status': doneCode} + return response.success() def _diskXMLGetVolumeChainInfo(self, diskXML, drive): def find_element_by_name(doc, name): -- To view, visit https://gerrit.ovirt.org/38270 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5fcd6c832f3a16a543357570c591c1f9a907c97a Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
