Skip to content
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.

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

blockVolume: check parent tag and meta match #285

Merged
merged 1 commit into from Jul 25, 2022

Conversation

aesteve-rh
Copy link
Member

@aesteve-rh aesteve-rh commented Jul 20, 2022

blockVolume getParent method uses the volume tag
PU_ to obtain the parent. However, this tag
could be outdated. For example, when the host
syncs a volume chain after removing a volume,
it only updates the metadata. Afterwards,
the SPM will update the volume tag once the
volume is deleted.

To ensure we do not return outdated parent UUID,
we need to ensure that the parent UUID obtained
from the parent tag, and the UUID in the volume
metadata match, and log a warning and update
the volume otherwise.

Tested with ovirt-stress: https://gitlab.com/nirs/ovirt-stress/-/merge_requests/14
It consistently triggered the bug in each run. After upgrading with this
branch patch, it passed each iteration (tried 10 iterations).

In the logs it can be seen:

2022-07-21 16:30:21,503+0200 WARN  (jsonrpc/5) [storage.volumemanifest] volUUID=3a34c1b2-70a5-4b25-bf66-b3f3f0e21c70 parent in tag (e5710d00-ed07-4ae6-bfbf-e1dd37a601e7) and metadata (4cf4d502-dfed-491a-960c-142ada884fdc) differ (blockVolume:172)
2022-07-21 16:30:21,565+0200 WARN  (jsonrpc/5) [storage.lvm] Removing stale lv: a2ad7a23-96c5-4c23-9007-c0d1892ea07d/e5710d00-ed07-4ae6-bfbf-e1dd37a601e7 (lvm:784)
2022-07-21 16:30:21,565+0200 DEBUG  (jsonrpc/5) [storage.volumemanifest] Parent tag after reload LVs, pUUID=4cf4d502-dfed-491a-960c-142ada884fdc (blockVolume:175)

Bug-Url: https://bugzilla.redhat.com/2103582
Signed-off-by: Albert Esteve aesteve@redhat.com

@aesteve-rh aesteve-rh requested a review from nirs as a code owner July 20, 2022 09:45
@aesteve-rh aesteve-rh requested a review from bennyz July 20, 2022 09:45
@aesteve-rh aesteve-rh self-assigned this Jul 20, 2022
@aesteve-rh aesteve-rh added bug Issue is a bug or fix for a bug storage labels Jul 20, 2022
@aesteve-rh aesteve-rh added this to the ovirt-4.5.2 milestone Jul 20, 2022
@aesteve-rh aesteve-rh added the verified Change was tested; please describe how it was tested in the PR label Jul 20, 2022
lib/vdsm/storage/blockVolume.py Outdated Show resolved Hide resolved
@nirs nirs requested a review from vjuranek July 20, 2022 19:07
@aesteve-rh aesteve-rh force-pushed the aesteve/lvm-cache-check-parent-tag branch from 0d02269 to 5b2ee4a Compare July 21, 2022 15:39
@aesteve-rh
Copy link
Member Author

/ost

lib/vdsm/storage/lvm.py Outdated Show resolved Hide resolved
lib/vdsm/storage/blockVolume.py Outdated Show resolved Hide resolved
lib/vdsm/storage/blockVolume.py Outdated Show resolved Hide resolved
lib/vdsm/storage/blockVolume.py Outdated Show resolved Hide resolved
@aesteve-rh aesteve-rh force-pushed the aesteve/lvm-cache-check-parent-tag branch 3 times, most recently from 189ffd9 to 5d91a73 Compare July 25, 2022 09:11
nirs
nirs previously approved these changes Jul 25, 2022
lib/vdsm/storage/blockVolume.py Outdated Show resolved Hide resolved
blockVolume getParent method uses the volume tag
PU_<id> to obtain the parent. However, this tag
could be outdated. For example, when the host
syncs a volume chain after removing a volume,
it only updates the metadata. Afterwards,
the SPM will update the volume tag once the
volume is deleted.

To ensure we do not return outdated parent UUID,
we need to ensure that the parent UUID obtained
from the parent tag, and the UUID in the volume
metadata match. If they do not, we try to reload
the volume tag first. If they still do not match
after that, log a warning and use the metadata parent.

Bug-Url: https://bugzilla.redhat.com/2103582
Signed-off-by: Albert Esteve <aesteve@redhat.com>
@nirs nirs force-pushed the aesteve/lvm-cache-check-parent-tag branch from 76a8111 to aeeaa81 Compare July 25, 2022 12:49
@aesteve-rh
Copy link
Member Author

/ost

@nirs nirs merged commit c61a5ef into oVirt:master Jul 25, 2022
@aesteve-rh aesteve-rh deleted the aesteve/lvm-cache-check-parent-tag branch September 12, 2022 08:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is a bug or fix for a bug storage verified Change was tested; please describe how it was tested in the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants