Skip to content

Fix GC hidden attribute in case of SIGTERM signal #760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix GC hidden attribute in case of SIGTERM signal #760

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 23 additions & 16 deletions libs/sm/cleanup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ def __init__(self, sr, uuid, raw):
self.sizeVirt = -1
self._sizeVHD = -1
self._sizeAllocated = -1
self.hidden = False
self._hidden = False
self.parent = None
self.children = []
self._vdiRef = None
Expand Down Expand Up @@ -638,15 +638,15 @@ def isCoalesceable(self):
return not self.scanError and \
self.parent and \
len(self.parent.children) == 1 and \
self.hidden and \
self.isHidden() and \
len(self.children) > 0

def isLeafCoalesceable(self):
"""A VDI is leaf-coalesceable if it has no siblings and is a leaf"""
return not self.scanError and \
self.parent and \
len(self.parent.children) == 1 and \
not self.hidden and \
not self.isHidden() and \
len(self.children) == 0

def canLiveCoalesce(self, speed):
Expand Down Expand Up @@ -674,7 +674,7 @@ def getAllPrunable(self):
# some tapdisks could still be using the file.
if self.sr.journaler.get(self.JRN_RELINK, self.uuid):
return []
if not self.scanError and self.hidden:
if not self.scanError and self.isHidden():
return [self]
return []

Expand Down Expand Up @@ -750,7 +750,7 @@ def delete(self):

def __str__(self):
strHidden = ""
if self.hidden:
if self.isHidden():
strHidden = "*"
strSizeVirt = "?"
if self.sizeVirt > 0:
Expand Down Expand Up @@ -1007,13 +1007,19 @@ def _setParent(self, parent):
Util.log("Failed to update %s with vhd-parent field %s" % \
(self.uuid, self.parentUuid))

def isHidden(self):
if self._hidden is None:
self._loadInfoHidden()
return self._hidden

def _loadInfoHidden(self):
hidden = vhdutil.getHidden(self.path)
self.hidden = (hidden != 0)
self._hidden = (hidden != 0)

def _setHidden(self, hidden=True):
self._hidden = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems this line is not necessary?

Copy link
Contributor

@MarkSymsCtx MarkSymsCtx Jun 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is as will trigger the code to re-read from the vhd. This is the core of the fix. It clears the state so that if the next call fails it will have to be read from the file.

vhdutil.setHidden(self.path, hidden)
self.hidden = hidden
self._hidden = hidden

def _increaseSizeVirt(self, size, atomic=True):
"""ensure the virtual size of 'self' is at least 'size'. Note that
Expand Down Expand Up @@ -1136,7 +1142,7 @@ def load(self, info=None):
self.sizeVirt = info.sizeVirt
self._sizeVHD = info.sizePhys
self._sizeAllocated = info.sizeAllocated
self.hidden = info.hidden
self._hidden = info.hidden
self.scanError = False
self.path = os.path.join(self.sr.path, "%s%s" % \
(self.uuid, vhdutil.FILE_EXTN_VHD))
Expand Down Expand Up @@ -1189,7 +1195,7 @@ def load(self, vdiInfo):
self.lvActive = vdiInfo.lvActive
self.lvOpen = vdiInfo.lvOpen
self.lvReadonly = vdiInfo.lvReadonly
self.hidden = vdiInfo.hidden
self._hidden = vdiInfo.hidden
self.parentUuid = vdiInfo.parentUuid
self.path = os.path.join(self.sr.path, self.fileName)

Expand Down Expand Up @@ -1314,14 +1320,15 @@ def _loadInfoSizeAllocated(self):

def _loadInfoHidden(self):
if self.raw:
self.hidden = self.sr.lvmCache.getHidden(self.fileName)
self._hidden = self.sr.lvmCache.getHidden(self.fileName)
else:
VDI._loadInfoHidden(self)

def _setHidden(self, hidden=True):
if self.raw:
self._hidden = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you do not have assign a None at here.

self.sr.lvmCache.setHidden(self.fileName, hidden)
self.hidden = hidden
self._hidden = hidden
else:
VDI._setHidden(self, hidden)

Expand All @@ -1330,7 +1337,7 @@ def __str__(self):
if self.raw:
strType = "RAW"
strHidden = ""
if self.hidden:
if self.isHidden():
strHidden = "*"
strSizeVHD = ""
if self._sizeVHD > 0:
Expand Down Expand Up @@ -2612,9 +2619,9 @@ def _undoInterruptedCoalesceLeaf(self, childUuid, parentUuid):
child.setConfig(VDI.DB_VDI_TYPE, vhdutil.VDI_TYPE_VHD)
util.fistpoint.activate("LVHDRT_coaleaf_undo_after_rename2", self.uuid)

if child.hidden:
if child.isHidden():
child._setHidden(False)
if not parent.hidden:
if not parent.isHidden():
parent._setHidden(True)
self._updateSlavesOnUndoLeafCoalesce(parent, child)
util.fistpoint.activate("LVHDRT_coaleaf_undo_end", self.uuid)
Expand Down Expand Up @@ -2827,9 +2834,9 @@ def _undoInterruptedCoalesceLeaf(self, childUuid, parentUuid):
parent.deflate()
child.inflateFully()
util.fistpoint.activate("LVHDRT_coaleaf_undo_after_deflate", self.uuid)
if child.hidden:
if child.isHidden():
child._setHidden(False)
if not parent.hidden:
if not parent.isHidden():
parent._setHidden(True)
if not parent.lvReadonly:
self.lvmCache.setReadonly(parent.fileName, True)
Expand Down
Loading