Revert "refactor: use VHD header to fetch block size"

This reverts commit baa7044.

Signed-off-by: Mark Syms <mark.syms@citrix.com>

Author: MarkSymsCtx

libs/sm/VDI.py

@@ -101,7 +101,6 @@ def __init__(self, sr, uuid):
         self.description = ''
         self.vbds = []
         self.size = 0
-        self._block_size = -1
         self.utilisation = 0
         self.vdi_type = ''
         self.has_child = 0
@@ -121,12 +120,6 @@ def __init__(self, sr, uuid):
 
         self.load(uuid)
 
-    @property
-    def block_size(self):
-        if self._block_size < 0:
-            self._block_size = vhdutil.getBlockSize(self.path)
-        return self._block_size
-
     @staticmethod
     def from_uuid(session, vdi_uuid):
 

libs/sm/cleanup.py

@@ -535,19 +535,12 @@ def __init__(self, sr, uuid, raw):
         self.sizeVirt = -1
         self._sizeVHD = -1
         self._sizeAllocated = -1
-        self._block_size = -1
         self._hidden = False
         self.parent = None
         self.children = []
         self._vdiRef = None
         self._clearRef()
 
-    @property
-    def block_size(self):
-        if self._block_size < 0:
-            self._block_size = vhdutil.getBlockSize(self.path)
-        return self._block_size
-
     @staticmethod
     def extractUuid(path):
         raise NotImplementedError("Implement in sub class")
@@ -1085,16 +1078,14 @@ def _getCoalescedSizeData(self):
         blocksParent = self.parent.getVHDBlocks()
         numBlocks = Util.countBits(blocksChild, blocksParent)
         Util.log("Num combined blocks = %d" % numBlocks)
-        sizeData = numBlocks * self.block_size
+        sizeData = numBlocks * vhdutil.VHD_BLOCK_SIZE
         assert(sizeData <= self.sizeVirt)
         return sizeData
 
     def _calcExtraSpaceForCoalescing(self):
         sizeData = self._getCoalescedSizeData()
-        sizeCoalesced = sizeData + vhdutil.calcOverheadBitmap(
-            sizeData,
-            self.block_size
-        ) + vhdutil.calcOverheadEmpty(self.sizeVirt)
+        sizeCoalesced = sizeData + vhdutil.calcOverheadBitmap(sizeData) + \
+                vhdutil.calcOverheadEmpty(self.sizeVirt)
         Util.log("Coalesced size = %s" % Util.num2str(sizeCoalesced))
         return sizeCoalesced - self.parent.getSizeVHD()
 
@@ -1250,7 +1241,7 @@ def deflate(self):
         self._sizeAllocated = -1
 
     def inflateFully(self):
-        self.inflate(lvhdutil.calcSizeVHDLV(self.sizeVirt, self.block_size))
+        self.inflate(lvhdutil.calcSizeVHDLV(self.sizeVirt))
 
     def inflateParentForCoalesce(self):
         """Inflate the parent only as much as needed for the purposes of
@@ -1476,10 +1467,7 @@ def _queryVHDBlocks(self):
     def _calcExtraSpaceForCoalescing(self):
         if self.parent.raw:
             return 0  # raw parents are never deflated in the first place
-        sizeCoalesced = lvhdutil.calcSizeVHDLV(
-            self._getCoalescedSizeData(),
-            self.block_size
-        )
+        sizeCoalesced = lvhdutil.calcSizeVHDLV(self._getCoalescedSizeData())
         Util.log("Coalesced size = %s" % Util.num2str(sizeCoalesced))
         return sizeCoalesced - self.parent.sizeLV
 
@@ -2821,7 +2809,7 @@ def _finishCoalesceLeaf(self, parent):
             parent.deflate()
 
     def _calcExtraSpaceNeeded(self, child, parent):
-        return lvhdutil.calcSizeVHDLV(parent.sizeVirt, parent.block_size) - parent.sizeLV
+        return lvhdutil.calcSizeVHDLV(parent.sizeVirt) - parent.sizeLV
 
     def _handleInterruptedCoalesceLeaf(self):
         entries = self.journaler.getAll(VDI.JRN_LEAF)

libs/sm/drivers/FileSR.py

@@ -656,7 +656,7 @@ def resize(self, sr_uuid, vdi_uuid, size):
             return VDI.VDI.get_params(self)
 
         # We already checked it is a VDI_TYPE_VHD
-        size = vhdutil.validate_and_round_vhd_size(int(size), self.block_size)
+        size = vhdutil.validate_and_round_vhd_size(int(size))
 
         jFile = JOURNAL_FILE_PREFIX + self.uuid
         try:

libs/sm/drivers/LVHDSR.py

@@ -721,10 +721,7 @@ def scan(self, uuid):
                                     util.roundup(lvutil.LVM_SIZE_INCREMENT,
                                       vhdutil.calcOverheadEmpty(lvhdutil.MSIZE))
                             else:
-                                utilisation = lvhdutil.calcSizeVHDLV(
-                                    int(size),
-                                    vhdutil.getBlockSize(lvPath)
-                                )
+                                utilisation = lvhdutil.calcSizeVHDLV(int(size))
 
                         vdi_ref = self.session.xenapi.VDI.db_introduce(
                                         vdi_uuid,
@@ -992,10 +989,7 @@ def _undoCloneOp(self, lvs, origUuid, baseUuid, clonUuid):
 
         # inflate the parent to fully-allocated size
         if base.vdiType == vhdutil.VDI_TYPE_VHD:
-            fullSize = lvhdutil.calcSizeVHDLV(
-                vhdInfo.sizeVirt,
-                vhdutil.getBlockSize(basePath)
-            )
+            fullSize = lvhdutil.calcSizeVHDLV(vhdInfo.sizeVirt)
             lvhdutil.inflate(self.journaler, self.uuid, baseUuid, fullSize)
 
         # rename back
@@ -1184,7 +1178,7 @@ def _undoAllVHDJournals(self):
             util.SMlog("Found VHD journal %s, reverting %s" % (uuid, vdi.path))
             self.lvActivator.activate(uuid, vdi.lvname, False)
             self.lvmCache.activateNoRefcount(jlvName)
-            fullSize = lvhdutil.calcSizeVHDLV(vdi.size, vdi.block_size)
+            fullSize = lvhdutil.calcSizeVHDLV(vdi.size)
             lvhdutil.inflate(self.journaler, self.uuid, vdi.uuid, fullSize)
             try:
                 jFile = os.path.join(self.path, jlvName)
@@ -1195,7 +1189,7 @@ def _undoAllVHDJournals(self):
                 util.SMlog("VHD revert failed but VHD ok: removing journal")
             # Attempt to reclaim unused space
             vhdInfo = vhdutil.getVHDInfo(vdi.path, lvhdutil.extractUuid, False)
-            NewSize = lvhdutil.calcSizeVHDLV(vhdInfo.sizeVirt, vdi.block_size)
+            NewSize = lvhdutil.calcSizeVHDLV(vhdInfo.sizeVirt)
             if NewSize < fullSize:
                 lvhdutil.deflate(self.lvmCache, vdi.lvname, int(NewSize))
             lvhdutil.lvRefreshOnAllSlaves(self.session, self.uuid,
@@ -1450,10 +1444,7 @@ def attach(self, sr_uuid, vdi_uuid):
             needInflate = False
         else:
             self._loadThis()
-            if (
-                self.utilisation >=
-                lvhdutil.calcSizeVHDLV(self.size, self.block_size)
-            ):
+            if self.utilisation >= lvhdutil.calcSizeVHDLV(self.size):
                 needInflate = False
 
         if needInflate:
@@ -1473,7 +1464,7 @@ def detach(self, sr_uuid, vdi_uuid):
         util.SMlog("LVHDVDI.detach for %s" % self.uuid)
         self._loadThis()
         already_deflated = (self.utilisation < \
-                lvhdutil.calcSizeVHDLV(self.size, self.block_size))
+                lvhdutil.calcSizeVHDLV(self.size))
         needDeflate = True
         if self.vdi_type == vhdutil.VDI_TYPE_RAW or already_deflated:
             needDeflate = False
@@ -1514,7 +1505,7 @@ def resize(self, sr_uuid, vdi_uuid, size):
                     '(current size: %d, new size: %d)' % (self.size, size))
             raise xs_errors.XenError('VDISize', opterr='shrinking not allowed')
 
-        size = vhdutil.validate_and_round_vhd_size(int(size), self.block_size)
+        size = vhdutil.validate_and_round_vhd_size(int(size))
 
         if size == self.size:
             return VDI.VDI.get_params(self)
@@ -1524,7 +1515,7 @@ def resize(self, sr_uuid, vdi_uuid, size):
             lvSizeNew = util.roundup(lvutil.LVM_SIZE_INCREMENT, size)
         else:
             lvSizeOld = self.utilisation
-            lvSizeNew = lvhdutil.calcSizeVHDLV(size, self.block_size)
+            lvSizeNew = lvhdutil.calcSizeVHDLV(size)
             if self.sr.provision == "thin":
                 # VDI is currently deflated, so keep it deflated
                 lvSizeNew = lvSizeOld
@@ -1690,7 +1681,7 @@ def _snapshot(self, snapType, cloneOp=False, cbtlog=None, cbt_consistency=None):
         self.issnap = self.session.xenapi.VDI.get_is_a_snapshot( \
                                                 self.sr.srcmd.params['vdi_ref'])
 
-        fullpr = lvhdutil.calcSizeVHDLV(self.size, self.block_size)
+        fullpr = lvhdutil.calcSizeVHDLV(self.size)
         thinpr = util.roundup(lvutil.LVM_SIZE_INCREMENT, \
                 vhdutil.calcOverheadEmpty(lvhdutil.MSIZE))
         lvSizeOrig = thinpr

libs/sm/lvhdutil.py

@@ -95,11 +95,11 @@ def calcSizeLV(sizeVHD):
     return util.roundup(LVM_SIZE_INCREMENT, sizeVHD)
 
 
-def calcSizeVHDLV(sizeVirt, block_size=vhdutil.DEFAULT_VHD_BLOCK_SIZE):
+def calcSizeVHDLV(sizeVirt):
     # all LVHD VDIs have the metadata area preallocated for the maximum
     # possible virtual size (for fast online VDI.resize)
     metaOverhead = vhdutil.calcOverheadEmpty(MSIZE)
-    bitmapOverhead = vhdutil.calcOverheadBitmap(sizeVirt, block_size)
+    bitmapOverhead = vhdutil.calcOverheadBitmap(sizeVirt)
     return calcSizeLV(sizeVirt + metaOverhead + bitmapOverhead)
 
 
@@ -208,12 +208,7 @@ def setSizeVirt(journaler, srUuid, vdiUuid, size, jFile):
     lvName = LV_PREFIX[vhdutil.VDI_TYPE_VHD] + vdiUuid
     vgName = VG_PREFIX + srUuid
     path = os.path.join(VG_LOCATION, vgName, lvName)
-    inflate(
-        journaler,
-        srUuid,
-        vdiUuid,
-        calcSizeVHDLV(size, vhdutil.getBlockSize(path))
-    )
+    inflate(journaler, srUuid, vdiUuid, calcSizeVHDLV(size))
     vhdutil.setSizeVirt(path, size, jFile)
 
 
@@ -238,8 +233,7 @@ def attachThin(journaler, srUuid, vdiUuid):
     _tryAcquire(lock)
     lvmCache.refresh()
     vhdInfo = vhdutil.getVHDInfoLVM(lvName, extractUuid, vgName)
-    path = os.path.join(VG_LOCATION, vgName, lvName)
-    newSize = calcSizeVHDLV(vhdInfo.sizeVirt, vhdutil.getBlockSize(path))
+    newSize = calcSizeVHDLV(vhdInfo.sizeVirt)
     currSizeLV = lvmCache.getSize(lvName)
     if newSize <= currSizeLV:
         return

libs/sm/vhdutil.py

@@ -30,7 +30,7 @@
 MAX_CHAIN_SIZE = 30  # max VHD parent chain size
 VHD_UTIL = "/usr/bin/vhd-util"
 OPT_LOG_ERR = "--debug"
-DEFAULT_VHD_BLOCK_SIZE = 2 * 1024 * 1024
+VHD_BLOCK_SIZE = 2 * 1024 * 1024
 VHD_FOOTER_SIZE = 512
 
 # lock to lock the entire SR for short ops
@@ -82,9 +82,9 @@ def calcOverheadEmpty(virtual_size):
     return overhead
 
 
-def calcOverheadBitmap(virtual_size, block_size=DEFAULT_VHD_BLOCK_SIZE):
-    num_blocks = virtual_size // block_size
-    if virtual_size % block_size:
+def calcOverheadBitmap(virtual_size):
+    num_blocks = virtual_size // VHD_BLOCK_SIZE
+    if virtual_size % VHD_BLOCK_SIZE:
         num_blocks += 1
     return num_blocks * 4096
 
@@ -93,18 +93,10 @@ def ioretry(cmd, text=True):
     return util.ioretry(lambda: util.pread2(cmd, text=text),
                         errlist=[errno.EIO, errno.EAGAIN])
 
-def getBlockSize(path):
-    cmd = [VHD_UTIL, "read", "-pn", path]
-    ret = ioretry(cmd)
-    for field in ret.split('\n'):
-        field = field.lstrip()
-        if not field.startswith("Block size"): continue
-        return int(field.split(':')[1].lstrip().split()[0])
-    raise util.SMException("Unable to find block size in VHD with path: {}".format(path))
-
 
-def convertAllocatedSizeToBytes(size, block_size=DEFAULT_VHD_BLOCK_SIZE):
-    return size * block_size
+def convertAllocatedSizeToBytes(size):
+    # Assume we have standard 2MB allocation blocks
+    return size * 2 * 1024 * 1024
 
 
 def getVHDInfo(path, extractUuidFunction, includeParent=True):
@@ -128,10 +120,7 @@ def getVHDInfo(path, extractUuidFunction, includeParent=True):
             vhdInfo.parentUuid = extractUuidFunction(fields[nextIndex])
         nextIndex += 1
     vhdInfo.hidden = int(fields[nextIndex].replace("hidden: ", ""))
-    vhdInfo.sizeAllocated = convertAllocatedSizeToBytes(
-        int(fields[nextIndex+1]),
-        getBlockSize(path)
-    )
+    vhdInfo.sizeAllocated = convertAllocatedSizeToBytes(int(fields[nextIndex+1]))
     vhdInfo.path = path
     return vhdInfo
 
@@ -290,7 +279,7 @@ def setSizePhys(path, size, debug=True):
 def getAllocatedSize(path):
     cmd = [VHD_UTIL, "query", OPT_LOG_ERR, '-a', '-n', path]
     ret = ioretry(cmd)
-    return convertAllocatedSizeToBytes(int(ret), getBlockSize(path))
+    return convertAllocatedSizeToBytes(int(ret))
 
 def killData(path):
     "zero out the disk (kill all data inside the VHD file)"
@@ -417,7 +406,7 @@ def repair(path):
     ioretry([VHD_UTIL, 'repair', '-n', path])
 
 
-def validate_and_round_vhd_size(size, block_size=DEFAULT_VHD_BLOCK_SIZE):
+def validate_and_round_vhd_size(size):
     """ Take the supplied vhd size, in bytes, and check it is positive and less
     that the maximum supported size, rounding up to the next block boundary
     """
@@ -430,7 +419,7 @@ def validate_and_round_vhd_size(size, block_size=DEFAULT_VHD_BLOCK_SIZE):
     if size < MIN_VHD_SIZE:
         size = MIN_VHD_SIZE
 
-    size = util.roundup(block_size, size)
+    size = util.roundup(VHD_BLOCK_SIZE, size)
 
     return size
 

tests/test_FileSR.py

@@ -327,7 +327,6 @@ def test_create_vdi_vhd(self, mock_vhdutil):
         vdi = FakeFileVDI(sr, vdi_uuid)
         vdi.vdi_type = vhdutil.VDI_TYPE_VHD
         mock_vhdutil.validate_and_round_vhd_size.side_effect = vhdutil.validate_and_round_vhd_size
-        mock_vhdutil.DEFAULT_VHD_BLOCK_SIZE = vhdutil.DEFAULT_VHD_BLOCK_SIZE
 
         # Act
         vdi.create(sr_uuid, vdi_uuid, 20 * 1024 * 1024)

tests/test_LVHDSR.py

@@ -566,13 +566,9 @@ def setUp(self):
         self.mock_lvhdutil.LV_PREFIX = lvhdutil.LV_PREFIX
         vhdutil_patcher = mock.patch('sm.drivers.LVHDSR.vhdutil', autospec=True)
         self.mock_vhdutil = vhdutil_patcher.start()
-        self.mock_vhdutil.getBlockSize.return_value = vhdutil.DEFAULT_VHD_BLOCK_SIZE
         self.mock_vhdutil.VDI_TYPE_VHD = vhdutil.VDI_TYPE_VHD
         self.mock_vhdutil.VDI_TYPE_RAW = vhdutil.VDI_TYPE_RAW
         self.mock_vhdutil.MAX_CHAIN_SIZE = vhdutil.MAX_CHAIN_SIZE
-        vdi_vhdutil_patcher = mock.patch('sm.VDI.vhdutil', autospec=True)
-        self.mock_vdi_vhdutil = vdi_vhdutil_patcher.start()
-        self.mock_vdi_vhdutil.getBlockSize.return_value = vhdutil.DEFAULT_VHD_BLOCK_SIZE
         lvutil_patcher = mock.patch('sm.drivers.LVHDSR.lvutil', autospec=True)
         self.mock_lvutil = lvutil_patcher.start()
         vdi_util_patcher = mock.patch('sm.VDI.util', autospec=True)

tests/test_vhdutil.py

@@ -376,14 +376,10 @@ def test_function(args, inp):
 
         context.add_executable(VHD_UTIL, test_function)
         from sm.drivers import FileSR
-        with unittest.mock.patch(
-                "sm.vhdutil.getBlockSize",
-                return_value=vhdutil.DEFAULT_VHD_BLOCK_SIZE
-        ):
-            vhdinfo = vhdutil.getVHDInfo(TEST_VHD_PATH, FileSR.FileVDI.extractUuid)
+        vhdinfo = vhdutil.getVHDInfo(TEST_VHD_PATH, FileSR.FileVDI.extractUuid)
 
-            # Act/Assert
-            self.assertEqual(18856*2*1024*1024 , vhdinfo.sizeAllocated)
+        # Act/Assert
+        self.assertEqual(18856*2*1024*1024 , vhdinfo.sizeAllocated)
 
     @testlib.with_context
     def test_get_allocated_size(self, context):
@@ -400,35 +396,11 @@ def test_function(args, inp):
         context.add_executable(VHD_UTIL, test_function)
 
         # Act
-        result = 0
-        with unittest.mock.patch(
-                "sm.vhdutil.getBlockSize",
-                return_value=vhdutil.DEFAULT_VHD_BLOCK_SIZE
-        ):
-            result = vhdutil.getAllocatedSize(TEST_VHD_NAME)
+        result = vhdutil.getAllocatedSize(TEST_VHD_NAME)
 
         # Assert
         self.assertEqual(18856*2*1024*1024, result)
         self.assertEqual(
             [VHD_UTIL, "query", "--debug", "-a",
              "-n", TEST_VHD_NAME],
             call_args)
-
-    @testlib.with_context
-    def test_get_block_size(self, context):
-        """
-        Test that vhdutil.getBlockSize returns the block size in bytes
-        """
-
-        # Arrange
-        call_args = None
-
-        def test_function(args, inp):
-            nonlocal call_args
-            call_args = args
-            return 0, "Header version: 0x00010000\nBlock size: {} (2 MB)".format(vhdutil.DEFAULT_VHD_BLOCK_SIZE), ""
-
-        context.add_executable(VHD_UTIL, test_function)
-
-        # Act/Assert
-        self.assertEqual(vhdutil.DEFAULT_VHD_BLOCK_SIZE, vhdutil.getBlockSize(TEST_VHD_NAME))