Change in vdsm[master]: Live Merge: Restore watermark tracking

fromani at redhat.com fromani at redhat.com
Mon Jan 26 09:40:21 UTC 2015


Francesco Romani has posted comments on this change.

Change subject: Live Merge: Restore watermark tracking
......................................................................


Patch Set 1:

(1 comment)

Thanks for the answer!

Do you plan for a tighter integration of this code with followup patches? By looking at the methods you added and how they are going to play with _getExtendCandidates, it _seems_ to me that there is some room for further improvement.

If softdeps are properly handled (I just need to read the updated commit message)it seems to me (IIUC all the details of the flow) that we could have just one loop in _getExtendCandidates and issue domainListGetStats here and once, and do all checks per-drive.
This could make the code easier to follow and spare us a lot of find*() and loops.

http://gerrit.ovirt.org/#/c/36924/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:

Line 1551:             return {}
Line 1552: 
Line 1553:         candidates = {}
Line 1554:         watermarks = self._getWriteWatermarks()
Line 1555:         for job in self.conf['_blockJobs'].values():
I wonder if it makes any sense to iterate not on blockJobs but on watermarks. IIUC the flow, this should save us to do again checks at line 1564 and 1582 below, since the not interesting drives are been ruled out by check at line 1534 above.

This could make the code a bit easier to follow.
Line 1556:             try:
Line 1557:                 drive = self._findDriveByUUIDs(job['disk'])
Line 1558:             except LookupError:
Line 1559:                 # After an active layer merge completes the vdsm metadata will


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I632f31e7795ec5d8c6f52a480116b14470c3163f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini 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