Nir Soffer has posted comments on this change.
Change subject: block: use the contextmanager for getVolumeMetadataSlot
......................................................................
Patch Set 6:
(1 comment)
https://gerrit.ovirt.org/#/c/25988/6/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:
Line 716:
Line 717: @contextmanager
Line 718: def getVolumeMetadataSlot(self, vol_name, slotSize):
Line 719: # TODO: _lvTagMetaSlotLock should be relevant only when we use
Line 720: # getFreeMetadataSlot to avoid a race between getting the occupied
We'll have to get Federico to weigh in on it.
Seems that
_lvTagMetaSlotLock should be used only when using getFreeMetadataSlot(), and not when
using getVolumeMetadataOffsetFromPvMapping().
When calling getFreeMetaDataSlot(), we must prevent adding or removing new slots when we
search for the occupied ones.
When calling getVolumeMetadataOffsetFromPvMapping() we seems to use the slot according to
the pv layout which cannot change, and mysterious "mapoffset" value which I
don't fully understand, added in downstream d282ec2e (BZ#675683, BZ#684584, BZ#664432
- Metadata refactoring).
I would replace the TODO with: "Check if the lock is needed when using
getVolumeMetadataOffsetFromPvMapping()", but I would not touch this code since it had
no tests, no documentation, and too many "tricks".
Line 721: # metadata slots and tagging the new LV (commit to the new slot).
Line 722: with self._lvTagMetaSlotLock:
Line 723: if self.getVersion() in VERS_METADATA_LV:
Line 724: yield self.getVolumeMetadataOffsetFromPvMapping(vol_name)
--
To view, visit
https://gerrit.ovirt.org/25988
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Id1496238ea4f9a1902f816b20959ce3379e87871
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Adam Litke <alitke(a)redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: automation(a)ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes