fix(cleanup.py): bad live coalesce check regarding FileSR

The `VDI.canLiveCoalesce` method can manipulates sizes of different units because of this change:
```
CP-40871: use VHD allocation size in checking canLiveCoalesce
2f863b9
```
As a result, the `canLiveCoalesce` method can return `True` and cause coalesce attempts
resulting in "Timed out" exceptions.

* Only drivers deriving from `FileSR` are impacted.

* The size of `self._sizeAllocated` is calculated correctly when `vhdutil.getAllocatedSize`
is called but there is a problematic case where `getVHDInfo` is used instead.
And this function does not convert `info.sizeAllocated` from block size to bytes.

* Additionally, the getter `getAllocatedSize` should retrieve the allocated size if it's equal to -1.
Otherwise `VDI.canLiveCoalesce` can compare -1 to a positive value and would always return `True`
causing a `Timed out` error error...

Signed-off-by: Damien Thenot <[email protected]>
Co-authored-by: Ronan Abhamon <[email protected]>

drivers/cleanup.py

@@ -1177,6 +1177,11 @@ def delete(self):
             raise util.SMException("os.unlink(%s) failed" % self.path)
         VDI.delete(self)
 
+    def getAllocatedSize(self):
+        if self._sizeAllocated == -1:
+            self._sizeAllocated = vhdutil.getAllocatedSize(self.path)
+        return self._sizeAllocated
+
 
 class LVHDVDI(VDI):
     """Object representing a VDI in an LVHD SR"""

drivers/vhdutil.py

@@ -94,6 +94,11 @@ def ioretry(cmd, text=True):
                         errlist=[errno.EIO, errno.EAGAIN])
 
 
+def convertAllocatedSizeToBytes(size):
+    # Assume we have standard 2MB allocation blocks
+    return size * 2 * 1024 * 1024
+
+
 def getVHDInfo(path, extractUuidFunction, includeParent=True):
     """Get the VHD info. The parent info may optionally be omitted: vhd-util
     tries to verify the parent by opening it, which results in error if the VHD
@@ -115,7 +120,7 @@ def getVHDInfo(path, extractUuidFunction, includeParent=True):
             vhdInfo.parentUuid = extractUuidFunction(fields[nextIndex])
         nextIndex += 1
     vhdInfo.hidden = int(fields[nextIndex].replace("hidden: ", ""))
-    vhdInfo.sizeAllocated = int(fields[nextIndex+1])
+    vhdInfo.sizeAllocated = convertAllocatedSizeToBytes(int(fields[nextIndex+1]))
     vhdInfo.path = path
     return vhdInfo
 
@@ -274,8 +279,7 @@ def setSizePhys(path, size, debug=True):
 def getAllocatedSize(path):
     cmd = [VHD_UTIL, "query", OPT_LOG_ERR, '-a', '-n', path]
     ret = ioretry(cmd)
-    # Assume we have standard 2MB allocation blocks
-    return int(ret) * 2 * 1024 * 1024
+    return convertAllocatedSizeToBytes(int(ret))
 
 def killData(path):
     "zero out the disk (kill all data inside the VHD file)"

tests/test_cleanup.py

@@ -1461,6 +1461,8 @@ def add_vdis_for_coalesce(self, sr):
         vdi = cleanup.FileVDI(sr, vdi_uuid, False)
         vdi.path = '%s.vhd' % (vdi_uuid)
         vdi.parent = parent
+        # Set an initial value to make Mock happy.
+        vdi._sizeAllocated = 20971520 # 10 blocks of 2MB changed in the child.
         parent.children.append(vdi)
 
         sr.vdis[vdi_uuid] = vdi

tests/test_vhdutil.py

@@ -362,3 +362,43 @@ def test_function(args, inp):
 
         # Act/Assert
         self.assertEqual(25, vhdutil.coalesce(TEST_VHD_PATH))
+
+    @testlib.with_context
+    def test_get_vhd_info_allocated_size(self, context):
+        """
+        Test that vhdutil.getVHDInfo return the allocated size in byte
+        """
+        # Arrange
+        def test_function(args, inp):
+            return 0, "51200\n39621239296\nd90f890c-d173-4eaf-ba09-fc2d6e50f6c0.vhd has no parent\nhidden: 0\n18856", ""
+
+        context.add_executable(VHD_UTIL, test_function)
+        import FileSR
+        vhdinfo = vhdutil.getVHDInfo(TEST_VHD_PATH, FileSR.FileVDI.extractUuid)
+
+        # Act/Assert
+        self.assertEqual(18856*2*1024*1024 , vhdinfo.sizeAllocated)
+
+    @testlib.with_context
+    def test_get_allocated_size(self, context):
+        """
+        Test that vhdutil.getAllocatedSize return the size in byte
+        """
+        # Arrange
+        call_args = None
+        def test_function(args, inp):
+            nonlocal call_args
+            call_args = args
+            return 0, b"18856", b""
+
+        context.add_executable(VHD_UTIL, test_function)
+
+        # Act
+        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)