Nir Soffer has posted comments on this change. Change subject: vm: Fix extend size calculation ......................................................................
Patch Set 2: (10 comments) http://gerrit.ovirt.org/#/c/37274/2/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 130: pass Line 131: Line 132: Line 133: class VolumeInfoError(RuntimeError): Line 134: pass Not needed, we use StorageUnavailableError bellow. Line 135: Line 136: Line 137: class BlockJobExistsError(Exception): Line 138: pass Line 1418: try: Line 1419: capacity, alloc, physical = self._getExtendInfo(drive) Line 1420: except (libvirt.libvirtError, VolumeInfoError) as e: Line 1421: self.log.error("Error getting extend info for %s: %s", Line 1422: drive.path, e) Should be in separate patch Line 1423: continue Line 1424: Line 1425: ret.append((drive, drive.volumeID, capacity, alloc, physical)) Line 1426: Line 1432: mergeCandidate['capacity'], mergeCandidate['alloc'], Line 1433: mergeCandidate['physical'])) Line 1434: return ret Line 1435: Line 1436: def _getExtendInfo(self, drive): _getDriveVolumeWatermarks? Line 1437: capacity, alloc, physical = self._dom.blockInfo(drive.path, 0) Line 1438: Line 1439: # When replicating file-based disk to block domain, physical is Line 1440: # identical to alloc, breaking extend logic. We can get the real Line 1447: replica["domainID"], replica["poolID"], replica["imageID"], Line 1448: replica["volumeID"]) Line 1449: if res['status']['code'] != 0: Line 1450: raise VolumeInfoError("Error getting volume info %s" % Line 1451: replica["volumeID"]) Not needed, use _getVolumeSize which raises if failed. Line 1452: physical = int(res['apparentsize']) Line 1453: Line 1454: return capacity, alloc, physical Line 1455: Line 1450: raise VolumeInfoError("Error getting volume info %s" % Line 1451: replica["volumeID"]) Line 1452: physical = int(res['apparentsize']) Line 1453: Line 1454: return capacity, alloc, physical Add VolumeWatermarks namedtuple for these, pass tuple instead of 3 arguments. Line 1455: Line 1456: def _shouldExtendVolume(self, drive, volumeID, capacity, alloc, physical): Line 1457: nextSize = vmdevices.storage.getNextVolumeSize( Line 1458: physical, capacity, drive.volExtensionChunk) Line 1455: Line 1456: def _shouldExtendVolume(self, drive, volumeID, capacity, alloc, physical): Line 1457: nextSize = vmdevices.storage.getNextVolumeSize( Line 1458: physical, capacity, drive.volExtensionChunk) Line 1459: nextPhysSize = nextSize * constants.MEGAB Not needed, master uses now bytes for all sizes. Line 1460: Line 1461: # NOTE: the intent of this check is to prevent faulty images to Line 1462: # trick qemu in requesting extremely large extensions (BZ#998443). Line 1463: # Probably the definitive check would be comparing the allocated Line 1509: return Line 1510: Line 1511: # TODO: Move back to Drive when we have a Replica object Line 1512: newSize = vmdevices.storage.getNextVolumeSize( Line 1513: curSize, capacity, vmDrive.volExtensionChunk) Not needed, in master getNextVolumeSize in in Drive Line 1514: Line 1515: replica = getattr(vmDrive, 'diskReplicate', {}) Line 1516: if replica.get("blockDev", False): Line 1517: # The replica is a block device. We will handle the drive itself in Line 3525: Line 3526: try: Line 3527: capacity, alloc, physical = self._getExtendInfo(srcDrive) Line 3528: self.extendDriveVolume(srcDrive, srcDrive.volumeID, physical, Line 3529: capacity) Already in master Line 3530: except Exception: Line 3531: self.log.exception("Initial extension request failed for %s", Line 3532: srcDrive.name) Line 3533: Line 4806: Line 4807: # If top is the active layer, it's allocated and maximum size are Line 4808: # stored in the drive. Line 4809: topSize = drive.apparentsize Line 4810: trueSize = drive.truesize Not needed, master use capacity from libvirt now. And this is also wrong - drive.truesize is a duplicated of drive.apparentsize Line 4811: else: Line 4812: # If top is an internal volume, we must call getVolumeInfo Line 4813: res = self.cif.irs.getVolumeInfo(drive.domainID, drive.poolID, Line 4814: drive.imageID, topVolUUID) Line 4847: # reported by libvirt so we must do the full extension up front. In Line 4848: # the worst case, we'll need to extend 'base' to the same size as 'top' Line 4849: # plus a bit more to accomodate additional writes to 'top' during the Line 4850: # live merge operation. Line 4851: self.extendDriveVolume(drive, baseVolUUID, topSize, trueSize) Alreay in master, using capacity from libvirt. Line 4852: Line 4853: # Trigger the collection of stats before returning so that callers Line 4854: # of getVmStats after this returns will see the new job Line 4855: self._vmStats.sampleVmJobs() -- To view, visit http://gerrit.ovirt.org/37274 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7afe3a7f6cfbb47eb76249d3851b7adaf4dbba6f Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[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
