From e88ba1a1c13c1eeae3d5d8479a03dbb711223d61 Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Tue, 19 Aug 2025 11:27:28 +0200 Subject: [PATCH 01/12] refactor: use VHD header to fetch block size Signed-off-by: Mathieu Labourier --- libs/sm/VDI.py | 7 +++++++ libs/sm/cleanup.py | 25 +++++++++++++++++++------ libs/sm/drivers/FileSR.py | 7 +++++-- libs/sm/drivers/LVHDSR.py | 37 ++++++++++++++++++++++++++----------- libs/sm/lvhdutil.py | 14 ++++++++++---- libs/sm/vhdutil.py | 32 ++++++++++++++++++++++---------- tests/test_vhdutil.py | 31 ++++++++++++++++++++++++------- 7 files changed, 113 insertions(+), 40 deletions(-) diff --git a/libs/sm/VDI.py b/libs/sm/VDI.py index 03c92737b..c2e679693 100644 --- a/libs/sm/VDI.py +++ b/libs/sm/VDI.py @@ -101,6 +101,7 @@ 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 @@ -120,6 +121,12 @@ 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): diff --git a/libs/sm/cleanup.py b/libs/sm/cleanup.py index 850086acd..b3e7139f3 100644 --- a/libs/sm/cleanup.py +++ b/libs/sm/cleanup.py @@ -535,12 +535,19 @@ 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") @@ -1075,14 +1082,16 @@ def _getCoalescedSizeData(self): blocksParent = self.parent.getVHDBlocks() numBlocks = Util.countBits(blocksChild, blocksParent) Util.log("Num combined blocks = %d" % numBlocks) - sizeData = numBlocks * vhdutil.VHD_BLOCK_SIZE + sizeData = numBlocks * self.block_size assert(sizeData <= self.sizeVirt) return sizeData def _calcExtraSpaceForCoalescing(self): sizeData = self._getCoalescedSizeData() - sizeCoalesced = sizeData + vhdutil.calcOverheadBitmap(sizeData) + \ - vhdutil.calcOverheadEmpty(self.sizeVirt) + sizeCoalesced = sizeData + vhdutil.calcOverheadBitmap( + sizeData, + self.block_size + ) + vhdutil.calcOverheadEmpty(self.sizeVirt) Util.log("Coalesced size = %s" % Util.num2str(sizeCoalesced)) return sizeCoalesced - self.parent.getSizeVHD() @@ -1238,7 +1247,7 @@ def deflate(self): self._sizeAllocated = -1 def inflateFully(self): - self.inflate(lvhdutil.calcSizeVHDLV(self.sizeVirt)) + self.inflate(lvhdutil.calcSizeVHDLV(self.sizeVirt, self.block_size)) def inflateParentForCoalesce(self): """Inflate the parent only as much as needed for the purposes of @@ -1461,7 +1470,10 @@ 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()) + sizeCoalesced = lvhdutil.calcSizeVHDLV( + self._getCoalescedSizeData(), + self.block_size + ) Util.log("Coalesced size = %s" % Util.num2str(sizeCoalesced)) return sizeCoalesced - self.parent.sizeLV @@ -2775,7 +2787,8 @@ def _finishCoalesceLeaf(self, parent): parent.deflate() def _calcExtraSpaceNeeded(self, child, parent): - return lvhdutil.calcSizeVHDLV(parent.sizeVirt) - parent.sizeLV + return lvhdutil.calcSizeVHDLV(parent.sizeVirt, parent.block_size) - \ + parent.sizeLV def _handleInterruptedCoalesceLeaf(self): entries = self.journaler.getAll(VDI.JRN_LEAF) diff --git a/libs/sm/drivers/FileSR.py b/libs/sm/drivers/FileSR.py index 75f7f9d15..68ec9402a 100644 --- a/libs/sm/drivers/FileSR.py +++ b/libs/sm/drivers/FileSR.py @@ -561,7 +561,10 @@ def create(self, sr_uuid, vdi_uuid, size): if self.vdi_type == vhdutil.VDI_TYPE_VHD: try: - size = vhdutil.validate_and_round_vhd_size(int(size)) + size = vhdutil.validate_and_round_vhd_size( + int(size), + vhdutil.VHD_BLOCK_SIZE + ) mb = 1024 * 1024 size_mb = size // mb util.ioretry(lambda: self._create(str(size_mb), self.path)) @@ -656,7 +659,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)) + size = vhdutil.validate_and_round_vhd_size(int(size), self.block_size) jFile = JOURNAL_FILE_PREFIX + self.uuid try: diff --git a/libs/sm/drivers/LVHDSR.py b/libs/sm/drivers/LVHDSR.py index f3ef89727..12a61fc5e 100644 --- a/libs/sm/drivers/LVHDSR.py +++ b/libs/sm/drivers/LVHDSR.py @@ -716,7 +716,10 @@ def scan(self, uuid): util.roundup(lvutil.LVM_SIZE_INCREMENT, vhdutil.calcOverheadEmpty(lvhdutil.MSIZE)) else: - utilisation = lvhdutil.calcSizeVHDLV(int(size)) + utilisation = lvhdutil.calcSizeVHDLV( + int(size), + vhdutil.getBlockSize(lvPath) + ) vdi_ref = self.session.xenapi.VDI.db_introduce( vdi_uuid, @@ -984,7 +987,10 @@ 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) + fullSize = lvhdutil.calcSizeVHDLV( + vhdInfo.sizeVirt, + vhdutil.getBlockSize(basePath) + ) lvhdutil.inflate(self.journaler, self.uuid, baseUuid, fullSize) # rename back @@ -1173,7 +1179,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) + fullSize = lvhdutil.calcSizeVHDLV(vdi.size, vdi.block_size) lvhdutil.inflate(self.journaler, self.uuid, vdi.uuid, fullSize) try: jFile = os.path.join(self.path, jlvName) @@ -1184,7 +1190,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) + NewSize = lvhdutil.calcSizeVHDLV(vhdInfo.sizeVirt, vdi.block_size) if NewSize < fullSize: lvhdutil.deflate(self.lvmCache, vdi.lvname, int(NewSize)) lvhdutil.lvRefreshOnAllSlaves(self.session, self.uuid, @@ -1343,7 +1349,10 @@ def create(self, sr_uuid, vdi_uuid, size): if self.exists: raise xs_errors.XenError('VDIExists') - size = vhdutil.validate_and_round_vhd_size(int(size)) + size = vhdutil.validate_and_round_vhd_size( + int(size), + vhdutil.VHD_BLOCK_SIZE + ) util.SMlog("LVHDVDI.create: type = %s, %s (size=%s)" % \ (self.vdi_type, self.path, size)) @@ -1356,7 +1365,10 @@ def create(self, sr_uuid, vdi_uuid, size): lvSize = util.roundup(lvutil.LVM_SIZE_INCREMENT, vhdutil.calcOverheadEmpty(lvhdutil.MSIZE)) elif self.sr.provision == "thick": - lvSize = lvhdutil.calcSizeVHDLV(int(size)) + lvSize = lvhdutil.calcSizeVHDLV( + int(size), + vhdutil.VHD_BLOCK_SIZE + ) self.sr._ensureSpaceAvailable(lvSize) @@ -1459,7 +1471,10 @@ def attach(self, sr_uuid, vdi_uuid): needInflate = False else: self._loadThis() - if self.utilisation >= lvhdutil.calcSizeVHDLV(self.size): + if ( + self.utilisation >= + lvhdutil.calcSizeVHDLV(self.size, self.block_size) + ): needInflate = False if needInflate: @@ -1479,7 +1494,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)) + lvhdutil.calcSizeVHDLV(self.size, self.block_size)) needDeflate = True if self.vdi_type == vhdutil.VDI_TYPE_RAW or already_deflated: needDeflate = False @@ -1520,7 +1535,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)) + size = vhdutil.validate_and_round_vhd_size(int(size), self.block_size) if size == self.size: return VDI.VDI.get_params(self) @@ -1530,7 +1545,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) + lvSizeNew = lvhdutil.calcSizeVHDLV(size, self.block_size) if self.sr.provision == "thin": # VDI is currently deflated, so keep it deflated lvSizeNew = lvSizeOld @@ -1696,7 +1711,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) + fullpr = lvhdutil.calcSizeVHDLV(self.size, self.block_size) thinpr = util.roundup(lvutil.LVM_SIZE_INCREMENT, \ vhdutil.calcOverheadEmpty(lvhdutil.MSIZE)) lvSizeOrig = thinpr diff --git a/libs/sm/lvhdutil.py b/libs/sm/lvhdutil.py index 71b5cbbb1..337f4aa4a 100644 --- a/libs/sm/lvhdutil.py +++ b/libs/sm/lvhdutil.py @@ -95,11 +95,11 @@ def calcSizeLV(sizeVHD): return util.roundup(LVM_SIZE_INCREMENT, sizeVHD) -def calcSizeVHDLV(sizeVirt): +def calcSizeVHDLV(sizeVirt, block_size): # 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) + bitmapOverhead = vhdutil.calcOverheadBitmap(sizeVirt, block_size) return calcSizeLV(sizeVirt + metaOverhead + bitmapOverhead) @@ -208,7 +208,12 @@ 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)) + inflate( + journaler, + srUuid, + vdiUuid, + calcSizeVHDLV(size, vhdutil.getBlockSize(path)) + ) vhdutil.setSizeVirt(path, size, jFile) @@ -233,7 +238,8 @@ def attachThin(journaler, srUuid, vdiUuid): _tryAcquire(lock) lvmCache.refresh() vhdInfo = vhdutil.getVHDInfoLVM(lvName, extractUuid, vgName) - newSize = calcSizeVHDLV(vhdInfo.sizeVirt) + path = os.path.join(VG_LOCATION, vgName, lvName) + newSize = calcSizeVHDLV(vhdInfo.sizeVirt, vhdutil.getBlockSize(path)) currSizeLV = lvmCache.getSize(lvName) if newSize <= currSizeLV: return diff --git a/libs/sm/vhdutil.py b/libs/sm/vhdutil.py index ccb5e57b4..e983b08bb 100644 --- a/libs/sm/vhdutil.py +++ b/libs/sm/vhdutil.py @@ -82,9 +82,9 @@ def calcOverheadEmpty(virtual_size): return overhead -def calcOverheadBitmap(virtual_size): - num_blocks = virtual_size // VHD_BLOCK_SIZE - if virtual_size % VHD_BLOCK_SIZE: +def calcOverheadBitmap(virtual_size, block_size): + num_blocks = virtual_size // block_size + if virtual_size % block_size: num_blocks += 1 return num_blocks * 4096 @@ -93,10 +93,19 @@ 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) + fields = ret.strip().split('\n') + for field in fields: + field = field.strip() + if not field.startswith("Block size"): continue + return int(field.split(':')[1].strip().split(' ')[0]) + return VHD_BLOCK_SIZE + -def convertAllocatedSizeToBytes(size): - # Assume we have standard 2MB allocation blocks - return size * 2 * 1024 * 1024 +def convertAllocatedSizeToBytes(size, block_size): + return size * block_size def getVHDInfo(path, extractUuidFunction, includeParent=True): @@ -120,7 +129,10 @@ 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])) + vhdInfo.sizeAllocated = convertAllocatedSizeToBytes( + int(fields[nextIndex+1]), + getBlockSize(path) + ) vhdInfo.path = path return vhdInfo @@ -279,7 +291,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)) + return convertAllocatedSizeToBytes(int(ret), getBlockSize(path)) def killData(path): "zero out the disk (kill all data inside the VHD file)" @@ -406,7 +418,7 @@ def repair(path): ioretry([VHD_UTIL, 'repair', '-n', path]) -def validate_and_round_vhd_size(size): +def validate_and_round_vhd_size(size, block_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 """ @@ -419,7 +431,7 @@ def validate_and_round_vhd_size(size): if size < MIN_VHD_SIZE: size = MIN_VHD_SIZE - size = util.roundup(VHD_BLOCK_SIZE, size) + size = util.roundup(block_size, size) return size diff --git a/tests/test_vhdutil.py b/tests/test_vhdutil.py index 0af601397..ab064a168 100644 --- a/tests/test_vhdutil.py +++ b/tests/test_vhdutil.py @@ -20,27 +20,38 @@ class TestVhdUtil(unittest.TestCase): def test_validate_and_round_min_size(self): - size = vhdutil.validate_and_round_vhd_size(2 * 1024 * 1024) + size = vhdutil.validate_and_round_vhd_size( + 2 * 1024 * 1024, + vhdutil.VHD_BLOCK_SIZE + ) self.assertTrue(size == 2 * 1024 * 1024) def test_validate_and_round_max_size(self): - size = vhdutil.validate_and_round_vhd_size(vhdutil.MAX_VHD_SIZE) + size = vhdutil.validate_and_round_vhd_size( + vhdutil.MAX_VHD_SIZE, + vhdutil.VHD_BLOCK_SIZE + ) self.assertTrue(size == vhdutil.MAX_VHD_SIZE) def test_validate_and_round_odd_size_up_to_next_boundary(self): - size = vhdutil.validate_and_round_vhd_size(vhdutil.MAX_VHD_SIZE - 1) + size = vhdutil.validate_and_round_vhd_size( + vhdutil.MAX_VHD_SIZE - 1, + vhdutil.VHD_BLOCK_SIZE) self.assertTrue(size == vhdutil.MAX_VHD_SIZE) def test_validate_and_round_negative(self): with self.assertRaises(xs_errors.SROSError): - vhdutil.validate_and_round_vhd_size(-1) + vhdutil.validate_and_round_vhd_size(-1, vhdutil.VHD_BLOCK_SIZE) def test_validate_and_round_too_large(self): with self.assertRaises(xs_errors.SROSError): - vhdutil.validate_and_round_vhd_size(vhdutil.MAX_VHD_SIZE + 1) + vhdutil.validate_and_round_vhd_size( + vhdutil.MAX_VHD_SIZE + 1, + vhdutil.VHD_BLOCK_SIZE + ) @testlib.with_context def test_calc_overhead_empty_small(self, context): @@ -65,14 +76,20 @@ def test_calc_overhead_empty_max(self, context): def test_calc_overhead_bitmap_round_blocks(self, context): virtual_size = 24 * 1024 * 1024 - result = vhdutil.calcOverheadBitmap(virtual_size) + result = vhdutil.calcOverheadBitmap( + virtual_size, + vhdutil.VHD_BLOCK_SIZE + ) self.assertEqual(49152, result) @testlib.with_context def test_calc_overhead_bitmap_extra_block(self, context): virtual_size = 25 * 1024 * 1024 - result = vhdutil.calcOverheadBitmap(virtual_size) + result = vhdutil.calcOverheadBitmap( + virtual_size, + vhdutil.VHD_BLOCK_SIZE + ) self.assertEqual(53248, result) From 85feea1cece89d1ad8ac6ae75f9d315d7fd6eefe Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Tue, 19 Aug 2025 12:22:30 +0200 Subject: [PATCH 02/12] fix: decode ret if necessary Signed-off-by: Mathieu Labourier --- libs/sm/vhdutil.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/libs/sm/vhdutil.py b/libs/sm/vhdutil.py index e983b08bb..5569c62ef 100644 --- a/libs/sm/vhdutil.py +++ b/libs/sm/vhdutil.py @@ -96,6 +96,12 @@ def ioretry(cmd, text=True): def getBlockSize(path): cmd = [VHD_UTIL, "read", "-pn", path] ret = ioretry(cmd) + if isinstance(ret, bytes): + import locale + ret = ret.decode( + encoding = locale.getpreferredencoding(False), + errors="replace" + ) fields = ret.strip().split('\n') for field in fields: field = field.strip() From bc713222b22953093bd9c6e4d07cc8110ec06984 Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Tue, 19 Aug 2025 12:28:48 +0200 Subject: [PATCH 03/12] fix: fallback to VHD_BLOCK_SIZE if error in fetch Signed-off-by: Mathieu Labourier --- libs/sm/vhdutil.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libs/sm/vhdutil.py b/libs/sm/vhdutil.py index 5569c62ef..069f658ee 100644 --- a/libs/sm/vhdutil.py +++ b/libs/sm/vhdutil.py @@ -95,7 +95,11 @@ def ioretry(cmd, text=True): def getBlockSize(path): cmd = [VHD_UTIL, "read", "-pn", path] - ret = ioretry(cmd) + try: + ret = ioretry(cmd) + except util.CommandException as e: + util.SMlog("WARN: unable to fetch block size: {}".format(e)) + return VHD_BLOCK_SIZE if isinstance(ret, bytes): import locale ret = ret.decode( From d8525ca8e842e7bfee3181937263db9aa2d153a9 Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Tue, 19 Aug 2025 12:38:43 +0200 Subject: [PATCH 04/12] fix(test_vhdutil): mock getBlockSize Signed-off-by: Mathieu Labourier --- tests/test_vhdutil.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_vhdutil.py b/tests/test_vhdutil.py index ab064a168..832a4ac0b 100644 --- a/tests/test_vhdutil.py +++ b/tests/test_vhdutil.py @@ -413,7 +413,12 @@ def test_function(args, inp): context.add_executable(VHD_UTIL, test_function) # Act - result = vhdutil.getAllocatedSize(TEST_VHD_NAME) + result = 0 + with unittest.mock.patch( + "sm.vhdutil.getBlockSize", + return_value=vhdutil.VHD_BLOCK_SIZE + ): + result = vhdutil.getAllocatedSize(TEST_VHD_NAME) # Assert self.assertEqual(18856*2*1024*1024, result) From 8f2905541a5b9d0cd6e6a193d1427ef157fa821a Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Tue, 19 Aug 2025 12:53:02 +0200 Subject: [PATCH 05/12] fix(test_FileSR): specify mock VHD_BLOCK_SIZE Signed-off-by: Mathieu Labourier --- tests/test_FileSR.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_FileSR.py b/tests/test_FileSR.py index 914549982..833dd2bfb 100644 --- a/tests/test_FileSR.py +++ b/tests/test_FileSR.py @@ -327,6 +327,7 @@ 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.VHD_BLOCK_SIZE = 2 * 1024 * 1024 # Act vdi.create(sr_uuid, vdi_uuid, 20 * 1024 * 1024) From fc386a416017a54daf4b7a10bc2d71ca35023776 Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Wed, 20 Aug 2025 11:00:37 +0200 Subject: [PATCH 06/12] style(cleanup): remove unnecessary line break in _calcExtraSpaceNeeded Signed-off-by: Mathieu Labourier --- libs/sm/cleanup.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libs/sm/cleanup.py b/libs/sm/cleanup.py index b3e7139f3..a6bcbedc7 100644 --- a/libs/sm/cleanup.py +++ b/libs/sm/cleanup.py @@ -2787,8 +2787,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.block_size) - parent.sizeLV def _handleInterruptedCoalesceLeaf(self): entries = self.journaler.getAll(VDI.JRN_LEAF) From 0ea97f563915a6ce8c173e9925fd754ff20b2212 Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Wed, 20 Aug 2025 11:02:40 +0200 Subject: [PATCH 07/12] refactor: rename VHD_BLOCK_SIZE to DEFAULT_VHD_BLOCK_SIZE Signed-off-by: Mathieu Labourier --- libs/sm/drivers/FileSR.py | 2 +- libs/sm/drivers/LVHDSR.py | 4 ++-- libs/sm/vhdutil.py | 6 +++--- tests/test_FileSR.py | 2 +- tests/test_vhdutil.py | 16 ++++++++-------- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/libs/sm/drivers/FileSR.py b/libs/sm/drivers/FileSR.py index 68ec9402a..5cb4601d3 100644 --- a/libs/sm/drivers/FileSR.py +++ b/libs/sm/drivers/FileSR.py @@ -563,7 +563,7 @@ def create(self, sr_uuid, vdi_uuid, size): try: size = vhdutil.validate_and_round_vhd_size( int(size), - vhdutil.VHD_BLOCK_SIZE + vhdutil.DEFAULT_VHD_BLOCK_SIZE ) mb = 1024 * 1024 size_mb = size // mb diff --git a/libs/sm/drivers/LVHDSR.py b/libs/sm/drivers/LVHDSR.py index 12a61fc5e..44bcbb4c0 100644 --- a/libs/sm/drivers/LVHDSR.py +++ b/libs/sm/drivers/LVHDSR.py @@ -1351,7 +1351,7 @@ def create(self, sr_uuid, vdi_uuid, size): size = vhdutil.validate_and_round_vhd_size( int(size), - vhdutil.VHD_BLOCK_SIZE + vhdutil.DEFAULT_VHD_BLOCK_SIZE ) util.SMlog("LVHDVDI.create: type = %s, %s (size=%s)" % \ @@ -1367,7 +1367,7 @@ def create(self, sr_uuid, vdi_uuid, size): elif self.sr.provision == "thick": lvSize = lvhdutil.calcSizeVHDLV( int(size), - vhdutil.VHD_BLOCK_SIZE + vhdutil.DEFAULT_VHD_BLOCK_SIZE ) self.sr._ensureSpaceAvailable(lvSize) diff --git a/libs/sm/vhdutil.py b/libs/sm/vhdutil.py index 069f658ee..9c3d4be7d 100644 --- a/libs/sm/vhdutil.py +++ b/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" -VHD_BLOCK_SIZE = 2 * 1024 * 1024 +DEFAULT_VHD_BLOCK_SIZE = 2 * 1024 * 1024 VHD_FOOTER_SIZE = 512 # lock to lock the entire SR for short ops @@ -99,7 +99,7 @@ def getBlockSize(path): ret = ioretry(cmd) except util.CommandException as e: util.SMlog("WARN: unable to fetch block size: {}".format(e)) - return VHD_BLOCK_SIZE + return DEFAULT_VHD_BLOCK_SIZE if isinstance(ret, bytes): import locale ret = ret.decode( @@ -111,7 +111,7 @@ def getBlockSize(path): field = field.strip() if not field.startswith("Block size"): continue return int(field.split(':')[1].strip().split(' ')[0]) - return VHD_BLOCK_SIZE + return DEFAULT_VHD_BLOCK_SIZE def convertAllocatedSizeToBytes(size, block_size): diff --git a/tests/test_FileSR.py b/tests/test_FileSR.py index 833dd2bfb..5b0913913 100644 --- a/tests/test_FileSR.py +++ b/tests/test_FileSR.py @@ -327,7 +327,7 @@ 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.VHD_BLOCK_SIZE = 2 * 1024 * 1024 + mock_vhdutil.DEFAULT_VHD_BLOCK_SIZE = vhdutil.DEFAULT_VHD_BLOCK_SIZE # Act vdi.create(sr_uuid, vdi_uuid, 20 * 1024 * 1024) diff --git a/tests/test_vhdutil.py b/tests/test_vhdutil.py index 832a4ac0b..f2bce40a4 100644 --- a/tests/test_vhdutil.py +++ b/tests/test_vhdutil.py @@ -22,7 +22,7 @@ class TestVhdUtil(unittest.TestCase): def test_validate_and_round_min_size(self): size = vhdutil.validate_and_round_vhd_size( 2 * 1024 * 1024, - vhdutil.VHD_BLOCK_SIZE + vhdutil.DEFAULT_VHD_BLOCK_SIZE ) self.assertTrue(size == 2 * 1024 * 1024) @@ -30,7 +30,7 @@ def test_validate_and_round_min_size(self): def test_validate_and_round_max_size(self): size = vhdutil.validate_and_round_vhd_size( vhdutil.MAX_VHD_SIZE, - vhdutil.VHD_BLOCK_SIZE + vhdutil.DEFAULT_VHD_BLOCK_SIZE ) self.assertTrue(size == vhdutil.MAX_VHD_SIZE) @@ -38,19 +38,19 @@ def test_validate_and_round_max_size(self): def test_validate_and_round_odd_size_up_to_next_boundary(self): size = vhdutil.validate_and_round_vhd_size( vhdutil.MAX_VHD_SIZE - 1, - vhdutil.VHD_BLOCK_SIZE) + vhdutil.DEFAULT_VHD_BLOCK_SIZE) self.assertTrue(size == vhdutil.MAX_VHD_SIZE) def test_validate_and_round_negative(self): with self.assertRaises(xs_errors.SROSError): - vhdutil.validate_and_round_vhd_size(-1, vhdutil.VHD_BLOCK_SIZE) + vhdutil.validate_and_round_vhd_size(-1, vhdutil.DEFAULT_VHD_BLOCK_SIZE) def test_validate_and_round_too_large(self): with self.assertRaises(xs_errors.SROSError): vhdutil.validate_and_round_vhd_size( vhdutil.MAX_VHD_SIZE + 1, - vhdutil.VHD_BLOCK_SIZE + vhdutil.DEFAULT_VHD_BLOCK_SIZE ) @testlib.with_context @@ -78,7 +78,7 @@ def test_calc_overhead_bitmap_round_blocks(self, context): result = vhdutil.calcOverheadBitmap( virtual_size, - vhdutil.VHD_BLOCK_SIZE + vhdutil.DEFAULT_VHD_BLOCK_SIZE ) self.assertEqual(49152, result) @@ -88,7 +88,7 @@ def test_calc_overhead_bitmap_extra_block(self, context): result = vhdutil.calcOverheadBitmap( virtual_size, - vhdutil.VHD_BLOCK_SIZE + vhdutil.DEFAULT_VHD_BLOCK_SIZE ) self.assertEqual(53248, result) @@ -416,7 +416,7 @@ def test_function(args, inp): result = 0 with unittest.mock.patch( "sm.vhdutil.getBlockSize", - return_value=vhdutil.VHD_BLOCK_SIZE + return_value=vhdutil.DEFAULT_VHD_BLOCK_SIZE ): result = vhdutil.getAllocatedSize(TEST_VHD_NAME) From 4c866c7dd0f862c3404b24ec2c2361852aa4b908 Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Wed, 20 Aug 2025 11:16:59 +0200 Subject: [PATCH 08/12] fix: adjust block size parsing fix: remove default value if error in fetch fix: raise an error instead of return default value if block size is not found Signed-off-by: Mathieu Labourier Co-authored-by: Ronan Abhamon Signed-off-by: Mathieu Labourier --- libs/sm/vhdutil.py | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/libs/sm/vhdutil.py b/libs/sm/vhdutil.py index 9c3d4be7d..5318def57 100644 --- a/libs/sm/vhdutil.py +++ b/libs/sm/vhdutil.py @@ -95,23 +95,18 @@ def ioretry(cmd, text=True): def getBlockSize(path): cmd = [VHD_UTIL, "read", "-pn", path] - try: - ret = ioretry(cmd) - except util.CommandException as e: - util.SMlog("WARN: unable to fetch block size: {}".format(e)) - return DEFAULT_VHD_BLOCK_SIZE + ret = ioretry(cmd) if isinstance(ret, bytes): import locale ret = ret.decode( encoding = locale.getpreferredencoding(False), errors="replace" ) - fields = ret.strip().split('\n') - for field in fields: - field = field.strip() + for field in ret.split('\n'): + field = field.lstrip() if not field.startswith("Block size"): continue - return int(field.split(':')[1].strip().split(' ')[0]) - return DEFAULT_VHD_BLOCK_SIZE + 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): From d0b64ead11c51bfa734f0ff6885c7b4a918ec465 Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Wed, 20 Aug 2025 11:23:53 +0200 Subject: [PATCH 09/12] fix(test_LVHDSR): mock getBlockSize Signed-off-by: Mathieu Labourier --- tests/test_LVHDSR.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_LVHDSR.py b/tests/test_LVHDSR.py index 20fda852d..0f4132816 100644 --- a/tests/test_LVHDSR.py +++ b/tests/test_LVHDSR.py @@ -307,9 +307,13 @@ 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) From 5c489feb8f2bcfe44d6a2f6a1742e9d26748a28f Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Wed, 20 Aug 2025 11:57:15 +0200 Subject: [PATCH 10/12] fix(vhdutil): remove bytes to str conversion Signed-off-by: Mathieu Labourier --- libs/sm/vhdutil.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/libs/sm/vhdutil.py b/libs/sm/vhdutil.py index 5318def57..f6348dda6 100644 --- a/libs/sm/vhdutil.py +++ b/libs/sm/vhdutil.py @@ -96,12 +96,6 @@ def ioretry(cmd, text=True): def getBlockSize(path): cmd = [VHD_UTIL, "read", "-pn", path] ret = ioretry(cmd) - if isinstance(ret, bytes): - import locale - ret = ret.decode( - encoding = locale.getpreferredencoding(False), - errors="replace" - ) for field in ret.split('\n'): field = field.lstrip() if not field.startswith("Block size"): continue From 4a7b747089d7b8ce4044780700c35834798b887c Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Wed, 20 Aug 2025 11:57:49 +0200 Subject: [PATCH 11/12] fix(test_vhdutil): mock getBlockSize in test_get_vhd_info_allocated_size Signed-off-by: Mathieu Labourier --- tests/test_vhdutil.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/test_vhdutil.py b/tests/test_vhdutil.py index f2bce40a4..43f4685a3 100644 --- a/tests/test_vhdutil.py +++ b/tests/test_vhdutil.py @@ -393,10 +393,14 @@ def test_function(args, inp): context.add_executable(VHD_UTIL, test_function) from sm.drivers import FileSR - vhdinfo = vhdutil.getVHDInfo(TEST_VHD_PATH, FileSR.FileVDI.extractUuid) + with unittest.mock.patch( + "sm.vhdutil.getBlockSize", + return_value=vhdutil.DEFAULT_VHD_BLOCK_SIZE + ): + 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): From d3a9a9dcf1a1bdc16a4ebace21070b90036f5f5b Mon Sep 17 00:00:00 2001 From: Mathieu Labourier Date: Wed, 20 Aug 2025 12:05:09 +0200 Subject: [PATCH 12/12] test(test_vhdutil): test getBlockSize Signed-off-by: Mathieu Labourier --- tests/test_vhdutil.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_vhdutil.py b/tests/test_vhdutil.py index 43f4685a3..d09abcd1e 100644 --- a/tests/test_vhdutil.py +++ b/tests/test_vhdutil.py @@ -430,3 +430,22 @@ def test_function(args, inp): [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))