Change in vdsm[master]: virt: Refactor drive extension
alitke at redhat.com
alitke at redhat.com
Mon Jun 9 20:11:25 UTC 2014
Adam Litke has uploaded a new change for review.
Change subject: virt: Refactor drive extension
......................................................................
virt: Refactor drive extension
A future live merge patch will require the ability to send extend
messages for internal drive volumes. To make this easier, refactor
the code a bit:
- Introduce a function that selects candidate volumes
- Introduce a function that chooses whether a volume should be extended
- getNextVolumeSize should take the current size as a parameter rather
than assuming it is self.apparentsize
- Pass volumeID explicitly to the extension functions since it may
differ from drive.volumeID
- When validating a resize, only update the drive if the leaf volume
was extended
Change-Id: I62b0941958618884aea67c7929ae3822a694ff5b
Signed-off-by: Adam Litke <alitke at redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 72 insertions(+), 49 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/33/28533/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index b154fbd..dd7636b 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -115,6 +115,11 @@
class DoubleDownError(RuntimeError):
pass
+
+class ImprobableResizeRequestError(RuntimeError):
+ pass
+
+
VALID_STATES = (vmstatus.DOWN, vmstatus.MIGRATION_DESTINATION,
vmstatus.MIGRATION_SOURCE, vmstatus.PAUSED,
vmstatus.POWERING_DOWN, vmstatus.REBOOT_IN_PROGRESS,
@@ -1149,14 +1154,16 @@
return (self.VOLWM_FREE_PCT * self.volExtensionChunk *
constants.MEGAB / 100)
- def getNextVolumeSize(self):
+ def getNextVolumeSize(self, curSize):
"""
Returns the next volume size in megabytes. This value is based on the
volExtensionChunk property and it's the size that should be requested
- for the next LV extension.
+ for the next LV extension. curSize is the current size of the volume
+ to be extended. For the leaf volume curSize == self.apparentsize.
+ For internal volumes it is discovered by calling irs.getVolumeSize().
"""
return (self.volExtensionChunk +
- ((self.apparentsize + constants.MEGAB - 1) / constants.MEGAB))
+ ((curSize + constants.MEGAB - 1) / constants.MEGAB))
@property
def networkDev(self):
@@ -2252,62 +2259,73 @@
with self._confLock:
self.conf['timeOffset'] = newTimeOffset
- def extendDrivesIfNeeded(self):
- extend = []
+ def _getExtendCandidates(self):
+ ret = []
for drive in self._devices[DISK_DEVICES]:
if not drive.blockDev or drive.format != 'cow':
continue
capacity, alloc, physical = self._dom.blockInfo(drive.path, 0)
+ ret.append((drive, drive.volumeID, capacity, alloc, physical))
+ return ret
- # Since the check based on nextPhysSize is extremly risky (it
- # may result in the VM being paused) we can't use the regular
- # getNextVolumeSize call as it relies on a cached value of the
- # drive apparentsize.
- nextPhysSize = physical + drive.VOLWM_CHUNK_MB * constants.MEGAB
+ def _shouldExtendVolume(self, drive, volumeID, capacity, alloc, physical):
+ # Since the check based on nextPhysSize is extremly risky (it
+ # may result in the VM being paused) we can't use the regular
+ # getNextVolumeSize call as it relies on a cached value of the
+ # drive apparentsize.
+ nextPhysSize = physical + drive.VOLWM_CHUNK_MB * constants.MEGAB
- # NOTE: the intent of this check is to prevent faulty images to
- # trick qemu in requesting extremely large extensions (BZ#998443).
- # Probably the definitive check would be comparing the allocated
- # space with capacity + format_overhead. Anyway given that:
- #
- # - format_overhead is tricky to be computed (it depends on few
- # assumptions that may change in the future e.g. cluster size)
- # - currently we allow only to extend by one chunk at time
- #
- # the current check compares alloc with the next volume size.
- # It should be noted that alloc cannot be directly compared with
- # the volume physical size as it includes also the clusters not
- # written yet (pending).
- if alloc > nextPhysSize:
- self.log.error(
- "Improbable extension request for volume %s on domain "
- "%s, pausing the VM to avoid corruptions (capacity: %s, "
- "allocated: %s, physical: %s, next physical size: %s)",
- drive.volumeID, drive.domainID, capacity, alloc,
- physical, nextPhysSize)
- self.pause(pauseCode='EOTHER')
- return False
+ # NOTE: the intent of this check is to prevent faulty images to
+ # trick qemu in requesting extremely large extensions (BZ#998443).
+ # Probably the definitive check would be comparing the allocated
+ # space with capacity + format_overhead. Anyway given that:
+ #
+ # - format_overhead is tricky to be computed (it depends on few
+ # assumptions that may change in the future e.g. cluster size)
+ # - currently we allow only to extend by one chunk at time
+ #
+ # the current check compares alloc with the next volume size.
+ # It should be noted that alloc cannot be directly compared with
+ # the volume physical size as it includes also the clusters not
+ # written yet (pending).
+ if alloc > nextPhysSize:
+ msg = ("Improbable extension request for volume %s on domain "
+ "%s, pausing the VM to avoid corruptions (capacity: %s, "
+ "allocated: %s, physical: %s, next physical size: %s)",
+ volumeID, drive.domainID, capacity, alloc,
+ physical, nextPhysSize)
+ self.log.error(msg)
+ self.pause(pauseCode='EOTHER')
+ raise ImprobableResizeRequestError(msg)
- if physical - alloc < drive.watermarkLimit:
- extend.append((drive, capacity, alloc, physical))
+ if physical - alloc < drive.watermarkLimit:
+ return True
+ return False
- for drive, capacity, alloc, physical in extend:
+ def extendDrivesIfNeeded(self):
+ try:
+ extend = [x for x in self._getExtendCandidates()
+ if self._shouldExtendVolume(*x)]
+ except ImprobableResizeRequestError:
+ return False
+
+ for drive, volumeID, capacity, alloc, physical in extend:
self.log.info(
"Requesting extension for volume %s on domain %s (apparent: "
"%s, capacity: %s, allocated: %s, physical: %s)",
- drive.volumeID, drive.domainID, drive.apparentsize, capacity,
+ volumeID, drive.domainID, drive.apparentsize, capacity,
alloc, physical)
- self.extendDriveVolume(drive)
+ self.extendDriveVolume(drive, volumeID, physical)
return len(extend) > 0
- def extendDriveVolume(self, vmDrive):
+ def extendDriveVolume(self, vmDrive, volumeID, curSize):
if not vmDrive.blockDev:
return
- newSize = vmDrive.getNextVolumeSize() # newSize is in megabytes
+ newSize = vmDrive.getNextVolumeSize(curSize) # newSize is in megabytes
if getattr(vmDrive, 'diskReplicate', None):
volInfo = {'poolID': vmDrive.diskReplicate['poolID'],
@@ -2321,7 +2339,7 @@
newSize * constants.MEGAB,
self.__afterReplicaExtension)
else:
- self.__extendDriveVolume(vmDrive, newSize)
+ self.__extendDriveVolume(vmDrive, volumeID, newSize)
def __refreshDriveVolume(self, volInfo):
self.cif.irs.refreshVolume(volInfo['domainID'], volInfo['poolID'],
@@ -2365,12 +2383,13 @@
self.log.debug("Requesting extension for the original drive: %s "
"(domainID: %s, volumeID: %s)",
vmDrive.name, vmDrive.domainID, vmDrive.volumeID)
- self.__extendDriveVolume(vmDrive, volInfo['newSize'])
+ self.__extendDriveVolume(vmDrive, vmDrive.volumeID, volInfo['newSize'])
- def __extendDriveVolume(self, vmDrive, newSize):
+ def __extendDriveVolume(self, vmDrive, volumeID, newSize):
volInfo = {'poolID': vmDrive.poolID, 'domainID': vmDrive.domainID,
- 'imageID': vmDrive.imageID, 'volumeID': vmDrive.volumeID,
- 'name': vmDrive.name, 'newSize': newSize}
+ 'imageID': vmDrive.imageID, 'volumeID': volumeID,
+ 'name': vmDrive.name, 'newSize': newSize,
+ 'internal': bool(vmDrive.volumeID != volumeID)}
self.log.debug("Requesting an extension for the volume: %s", volInfo)
self.cif.irs.sendExtendMsg(
vmDrive.poolID,
@@ -2379,12 +2398,15 @@
self.__afterVolumeExtension)
def __afterVolumeExtension(self, volInfo):
- # Either the extension succeeded and we're setting the new apparentSize
- # and trueSize, or it fails and we raise an exception.
+ # Check if the extension succeeded. On failure an exception is raised
# TODO: Report failure to the engine.
apparentSize, trueSize = self.__verifyVolumeExtension(volInfo)
- vmDrive = self._findDriveByName(volInfo['name'])
- vmDrive.apparentsize, vmDrive.truesize = apparentSize, trueSize
+
+ # Only update apparentsize and truesize if we've resized the leaf
+ if not volInfo['internal']:
+ vmDrive = self._findDriveByName(volInfo['name'])
+ vmDrive.apparentsize, vmDrive.truesize = apparentSize, trueSize
+
try:
self.cont()
except libvirt.libvirtError:
@@ -4055,7 +4077,8 @@
return errCode['replicaErr']
try:
- self.extendDriveVolume(srcDrive)
+ self.extendDriveVolume(srcDrive, srcDrive.volumeID,
+ srcDrive.apparentsize)
except Exception:
self.log.error("Initial extension request failed for %s",
srcDrive.name, exc_info=True)
--
To view, visit http://gerrit.ovirt.org/28533
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I62b0941958618884aea67c7929ae3822a694ff5b
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
More information about the vdsm-patches
mailing list