Change in vdsm[master]: janitorial: add the response.success() helper

fromani at redhat.com fromani at redhat.com
Mon Mar 2 10:25:18 UTC 2015


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 <fromani at redhat.com>
---
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 <fromani at redhat.com>


More information about the vdsm-patches mailing list