Change in vdsm[master]: Live Merge: support active layer commit

alitke at redhat.com alitke at redhat.com
Mon Jun 30 19:09:43 UTC 2014


Adam Litke has posted comments on this change.

Change subject: Live Merge: support active layer commit
......................................................................


Patch Set 5: -Verified

(5 comments)

http://gerrit.ovirt.org/#/c/28598/5/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 5541:                 flags |= libvirt.VIR_DOMAIN_BLOCK_COMMIT_ACTIVE
Line 5542:             except AttributeError:
Line 5543:                 self.log.error("Unable to merge the active layer.  Libvirt is "
Line 5544:                                "missing VIR_DOMAIN_BLOCK_COMMIT_ACTIVE.")
Line 5545:                 return errCode['mergeErr']
> I think we can factor out this code with the new one added into http://gerr
will do.
Line 5546: 
Line 5547:         try:
Line 5548:             self.trackBlockJob(jobUUID, drive, baseVolUUID, topVolUUID,
Line 5549:                                'commit')


Line 5666:                     if x['volumeID'] in volumes]
Line 5667:         device['volumeChain'] = drive.volumeChain = newChain
Line 5668: 
Line 5669:     def _onBlockJobActiveCommitEvent(self, diskName):
Line 5670:         flags = libvirt.VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT
> this temporary looks unnecessary
This is a pep8 workaround since I could not get the code to look nice in 79 columns without doing it this way.
Line 5671:         try:
Line 5672:             self._dom.blockJobAbort(diskName, flags)
Line 5673:         except libvirt.libvirtError:
Line 5674:             self.log.error("Failed to perform pivot", exc_info=True)


Line 5670:         flags = libvirt.VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT
Line 5671:         try:
Line 5672:             self._dom.blockJobAbort(diskName, flags)
Line 5673:         except libvirt.libvirtError:
Line 5674:             self.log.error("Failed to perform pivot", exc_info=True)
> self._reportException may be handy here
ok, will investigate.
Line 5675: 
Line 5676:     def _onBlockJobEvent(self, diskName, blockJobType, status):
Line 5677:         def isActiveCommit(jobType, status):
Line 5678:             if status != libvirt.VIR_DOMAIN_BLOCK_JOB_READY:


Line 5681:                 if jobType == libvirt.VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT:
Line 5682:                     return True
Line 5683:             except AttributeError:
Line 5684:                 pass
Line 5685:             return False
> Same as per line 5524: can we simplify this helper function , maybe after r
ok.
Line 5686: 
Line 5687:         drive = self._findDriveByName(diskName)
Line 5688:         try:
Line 5689:             jobID = self.getBlockJob(drive)['jobID']


Line 5686: 
Line 5687:         drive = self._findDriveByName(diskName)
Line 5688:         try:
Line 5689:             jobID = self.getBlockJob(drive)['jobID']
Line 5690:         except LookupError:
> Just caught my eye - can this raise KeyError also?
No.  It should not be possible.
Line 5691:             self.log.debug("Ignoring event for untracked block job on disk "
Line 5692:                            "'%s'", diskName)
Line 5693:             return
Line 5694: 


-- 
To view, visit http://gerrit.ovirt.org/28598
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Idab76081c2d004bc4f9e5bc9cb72e86845640f6a
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list