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

fix(blobstore): snapshot removal now properly ordered upon blobstore delete #32

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

mtzaurus
Copy link

Snapshots are now always deleted after all existing non-snapshot lvols when removing a blobstore.

…delete

Snapshots are now always deleted after all existing non-snapshot lvols when
removing a blobstore.

Signed-off-by: Mikhail Tcymbaliuk <[email protected]>
@mtzaurus mtzaurus marked this pull request as ready for review July 14, 2023 11:12
case DELETE_LVS:
if (!TAILQ_EMPTY(&lvs->lvols)) {
SPDK_ERRLOG("Lvols left in lvs ('%s'), but unable to delete.\n", lvs->name);
assert(false);

Choose a reason for hiding this comment

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

Can we handle this more gracefully, complete with an error rather than assert?

Copy link
Author

Choose a reason for hiding this comment

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

I tried to preserve original logic here: if this assertion triggers, this means that forced lvol removal doesn't work and it might be a symptom of a problem.

Choose a reason for hiding this comment

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

I see, though crashing will bring everything down only to crash again next time we try to delete?

Copy link
Author

Choose a reason for hiding this comment

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

But maybe you. are right - let's be not too strict and issue warning.

@mtzaurus mtzaurus merged commit 7ca587a into v23.01.x-mayastor Jul 17, 2023
1 of 2 checks passed
@mtzaurus mtzaurus deleted the fix/GTM-1102 branch July 17, 2023 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants