Skip to content

Commit

Permalink
fix(cleanup.py): bad live coalesce check regarding FileSR
Browse files Browse the repository at this point in the history
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.
This bug is caused by the call to `FileVDI.load` in cleanup.py.

When `FileVDI.load` is called with vhdinfo obtained from a scan,
`info.sizeAllocated` is given as -1.
Change `FileVDI.getAllocatedSize` to obtain the correct value with
`vhdutil.getAllocatedSize` in the case it is not already given.
Otherwise, `canLiveCoalesce` would always return `True` since it would
compare with `-1` and the leaf coalesce could fail with TimedOut error
systematically.

Signed-off-by: Damien Thenot <damien.thenot@vates.tech>
Co-authored-by: Ronan Abhamon <ronan.abhamon@vates.tech>
  • Loading branch information
Nambrok and Wescoeur committed Nov 27, 2024
1 parent 6d2b2d1 commit 010ed5a
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 3 deletions.
5 changes: 5 additions & 0 deletions drivers/cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -1193,6 +1193,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"""
Expand Down
10 changes: 7 additions & 3 deletions drivers/vhdutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, resolveParent=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
Expand All @@ -118,7 +123,7 @@ def getVHDInfo(path, extractUuidFunction, includeParent=True, resolveParent=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

Expand Down Expand Up @@ -277,8 +282,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)"
Expand Down
1 change: 1 addition & 0 deletions tests/test_cleanup.py
Original file line number Diff line number Diff line change
Expand Up @@ -1365,6 +1365,7 @@ def add_vdis_for_coalesce(self, sr):
vdi = cleanup.FileVDI(sr, vdi_uuid, False)
vdi.path = '%s.vhd' % (vdi_uuid)
vdi.parent = parent
vdi._sizeAllocated = 20971520 #10 blocks of 2MB changed in the child
parent.children.append(vdi)

sr.vdis[vdi_uuid] = vdi
Expand Down

0 comments on commit 010ed5a

Please sign in to comment.