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