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 <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
