Change in vdsm[master]: Live Merge: Simplify pivot flow

Federico Simoncelli fsimonce at redhat.com
Tue Sep 2 11:51:08 UTC 2014


Federico Simoncelli has posted comments on this change.

Change subject: Live Merge: Simplify pivot flow
......................................................................


Patch Set 5:

(3 comments)

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

Line 5365:                 else:
Line 5366:                     # Libvirt has stopped reporting this job so we know it will
Line 5367:                     # never report it again.
Line 5368:                     doPivot = False
Line 5369:                     storedJob['gone'] = True
A whitespace here would have helped to understand that we're entering in another new if block (not related to the previous if/else)
Line 5370:                 if not liveInfo or doPivot:
Line 5371:                     if not cleanThread:
Line 5372:                         # There is no cleanup thread so the job must have just
Line 5373:                         # ended.  Spawn an async cleanup.


Line 5636:         # active layer contains the same data as its parent so the ILLEGAL
Line 5637:         # flag indicates that the VM should be restarted using the parent.
Line 5638:         newVols = [vol['volumeID'] for vol in self.drive.volumeChain
Line 5639:                    if vol['volumeID'] != self.drive.volumeID]
Line 5640:         self.vm.cif.irs.imageSyncVolumeChain(self.drive.domainID,
I remember that we agreed on using only one irs command (imageSyncVolumeChain) because it would have been called in one place only from vm.py.

Instead it seems that now we're calling it from different places knowing in advance if it should pivot or not.
Line 5641:                                              self.drive.imageID,
Line 5642:                                              self.drive['volumeID'], newVols)
Line 5643: 
Line 5644:         self.vm.log.info("Requesting pivot to complete active layer commit "


Line 5653:         if self.doPivot:
Line 5654:             self.tryPivot()
Line 5655:         self.vm.log.info("Synchronizing volume chain after live merge "
Line 5656:                          "(job %s)", self.jobId)
Line 5657:         self.vm._syncVolumeChain(self.drive)
Is this going to call imageSyncVolumeChain once again?

Is there anything of the two flows that we can unify?
Line 5658:         self.success = True
Line 5659:         self.vm.log.info("Synchronization completed (job %s)", self.jobId)
Line 5660: 
Line 5661:     def isSuccessful(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I5e6a9ea7096b5d81f418754d411f135450bf572e
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: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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