Skip to content

Commit

Permalink
fix(LinstorSR): ensure _is_master is always set
Browse files Browse the repository at this point in the history
`_is_master` is not always initialized, and more precisely
in the case of detach where LinstorSR.load method is not called.

Still in this same situation, this can lead to the deletion of DRBD diskless
on the master while we try to always have one so as not to needlessly
recreate one later.

Signed-off-by: Ronan Abhamon <ronan.abhamon@vates.fr>
  • Loading branch information
Wescoeur committed Jul 26, 2024
1 parent 8c6fe49 commit 8bf754b
Showing 1 changed file with 20 additions and 15 deletions.
35 changes: 20 additions & 15 deletions drivers/LinstorSR.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,9 +362,6 @@ def load(self, sr_uuid):
self._linstor = None # Ensure that LINSTOR attribute exists.
self._journaler = None

self._is_master = False
if 'SRmaster' in self.dconf and self.dconf['SRmaster'] == 'true':
self._is_master = True
self._group_name = self.dconf['group-name']

self._vdi_shared_time = 0
Expand Down Expand Up @@ -437,7 +434,7 @@ def connect():

return wrapped_method(self, *args, **kwargs)

if not self._is_master:
if not self.is_master():
if self.cmd in [
'sr_create', 'sr_delete', 'sr_update', 'sr_probe',
'sr_scan', 'vdi_create', 'vdi_delete', 'vdi_resize',
Expand Down Expand Up @@ -472,7 +469,7 @@ def connect():

# Ensure we use a non-locked volume when vhdutil is called.
if (
self._is_master and self.cmd.startswith('vdi_') and
self.is_master() and self.cmd.startswith('vdi_') and
self.cmd != 'vdi_create'
):
self._linstor.ensure_volume_is_not_locked(
Expand All @@ -487,7 +484,7 @@ def connect():
#
# If the command is a SR command we want at least to remove
# resourceless volumes.
if self._is_master and self.cmd not in [
if self.is_master() and self.cmd not in [
'vdi_attach', 'vdi_detach',
'vdi_activate', 'vdi_deactivate',
'vdi_epoch_begin', 'vdi_epoch_end',
Expand Down Expand Up @@ -783,6 +780,14 @@ def scan(self, uuid):
self._kick_gc()
return ret

def is_master(self):
if 'SRmaster' not in self.dconf:
self._is_master = self.session is not None and util.is_master(self.session)
else:
self._is_master = self.dconf['SRmaster'] == 'true'

return self._is_master

@_locked_load
def vdi(self, uuid):
return LinstorVDI(self, uuid)
Expand Down Expand Up @@ -968,7 +973,7 @@ def _synchronize_metadata_and_xapi(self):
)

def _synchronize_metadata(self):
if not self._is_master:
if not self.is_master():
return

util.SMlog('Synchronize metadata...')
Expand Down Expand Up @@ -1015,7 +1020,7 @@ def _load_vdis(self):
if self._vdis_loaded:
return

assert self._is_master
assert self.is_master()

# We use a cache to avoid repeated JSON parsing.
# The performance gain is not big but we can still
Expand Down Expand Up @@ -1494,7 +1499,7 @@ def _reconnect(self):
controller_uri,
self._group_name,
repair=(
self._is_master and
self.is_master() and
self.srcmd.cmd in self.ops_exclusive
),
logger=util.SMlog
Expand Down Expand Up @@ -1796,7 +1801,7 @@ def attach(self, sr_uuid, vdi_uuid):
writable = 'args' not in self.sr.srcmd.params or \
self.sr.srcmd.params['args'][0] == 'true'

if not attach_from_config or self.sr._is_master:
if not attach_from_config or self.sr.is_master():
# We need to inflate the volume if we don't have enough place
# to mount the VHD image. I.e. the volume capacity must be greater
# than the VHD size + bitmap size.
Expand Down Expand Up @@ -1878,7 +1883,7 @@ def detach(self, sr_uuid, vdi_uuid):
)

# We remove only on slaves because the volume can be used by the GC.
if self.sr._is_master:
if self.sr.is_master():
return

while vdi_uuid:
Expand All @@ -1899,7 +1904,7 @@ def detach(self, sr_uuid, vdi_uuid):

def resize(self, sr_uuid, vdi_uuid, size):
util.SMlog('LinstorVDI.resize for {}'.format(self.uuid))
if not self.sr._is_master:
if not self.sr.is_master():
raise xs_errors.XenError(
'VDISize',
opterr='resize on slave not allowed'
Expand Down Expand Up @@ -2158,7 +2163,7 @@ def update(self, sr_uuid, vdi_uuid):
# --------------------------------------------------------------------------

def _prepare_thin(self, attach):
if self.sr._is_master:
if self.sr.is_master():
if attach:
attach_thin(
self.session, self.sr._journaler, self._linstor,
Expand Down Expand Up @@ -2747,7 +2752,7 @@ def _attach_using_http_nbd(self):

# 0. Fetch drbd path.
must_get_device_path = True
if not self.sr._is_master:
if not self.sr.is_master():
# We are on a slave, we must try to find a diskful locally.
try:
volume_info = self._linstor.get_volume_info(self.uuid)
Expand All @@ -2762,7 +2767,7 @@ def _attach_using_http_nbd(self):
must_get_device_path = hostname in volume_info.diskful

drbd_path = None
if must_get_device_path or self.sr._is_master:
if must_get_device_path or self.sr.is_master():
# If we are master, we must ensure we have a diskless
# or diskful available to init HA.
# It also avoid this error in xensource.log
Expand Down

0 comments on commit 8bf754b

Please sign in to comment.