Nir Soffer has posted comments on this change.
Change subject: Live Merge: Restore watermark tracking
......................................................................
Patch Set 2:
(5 comments)
Mostly ok, but will have to invest more time in this. Added few comments and questions.
http://gerrit.ovirt.org/#/c/36924/2/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 172: # block statistics for all volumes in the chain when using a new flag.
Line 173: _libvirtBackingChainStatsFlag = \
Line 174: libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING
Line 175: except AttributeError:
Line 176: _libvirtBackingChainStatsFlag = 0
This will be little nicer:
_STATS_BACKING_FLAG = getattr(
libvirt, "VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING", 0)
Line 177:
Line 178:
Line 179: class VmStatsThread(AdvancedStatsThread):
Line 180: MBPS_TO_BPS = 10 ** 6 / 8
Line 1431: return vol['volumeID']
Line 1432: raise LookupError("Unable to find VolumeID for path
'%s'", path)
Line 1433:
Line 1434: volAllocMap = {}
Line 1435: blkStats = self._getBulkStats(libvirt.VIR_DOMAIN_STATS_BLOCK,
Can we call this bulkStats?
Line 1436: _libvirtBackingChainStatsFlag)
Line 1437: for i in xrange(blkStats['block.count']):
Line 1438: name = blkStats['block.%i.name' % i]
Line 1439: try:
Line 4763: startCleanup(storedJob, drive, doPivot)
Line 4764: jobsRet[jobID] = entry
Line 4765: return jobsRet
Line 4766:
Line 4767: def _libvirtBackingChainStatsFlag(self):
Can be deleted
Line 4768: # Since libvirt 1.2.13, the virConnectGetAllDomainStats API will
return
Line 4769: # block statistics for all volumes in the chain when using a new flag.
Line 4770: try:
Line 4771: return libvirt.VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING
Line 4873: # during the rest of the operation. Otherwise, extend now to
Line 4874: # accomodate the worst case scenario: no intersection between the
Line 4875: # allocated blocks in the base volume and the top volume.
Line 4876: if drive.blockDev and baseCow:
Line 4877: if self._libvirtBackingChainStatsFlag():
Replace with the constant
Line 4878: self.extendDrivesIfNeeded()
Line 4879: else:
Line 4880: extendSize = baseSize + topSize
Line 4881: self.log.debug("Preemptively extending volume %s with size
%i"
Line 4874: # accomodate the worst case scenario: no intersection between the
Line 4875: # allocated blocks in the base volume and the top volume.
Line 4876: if drive.blockDev and baseCow:
Line 4877: if self._libvirtBackingChainStatsFlag():
Line 4878: self.extendDrivesIfNeeded()
This will not extend by one chunk as you describe, but try to extend all drives, and will
probably do not extend this drive.
I think we should calculate the requested size in the if, and extend out of the if.
Line 4879: else:
Line 4880: extendSize = baseSize + topSize
Line 4881: self.log.debug("Preemptively extending volume %s with size
%i"
Line 4882: "(job: %s)", baseVolUUID, extendSize,
jobUUID)
--
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes