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

Zero metadata superblock on detach #863

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

Deixx
Copy link
Contributor

@Deixx Deixx commented Jan 30, 2025

Zero superblock instead of setting the shutdown status.
Metadata from a detached cache device is not meant to be loaded.

ocf_metadata_set_shutdown_status(cache, ocf_metadata_detached,
_ocf_mngt_cache_unplug_complete, context);
/* Skip metadata update - will be zeroed later in detach pipeline */
ocf_pipeline_next(ctx->pipeline);
Copy link
Member

Choose a reason for hiding this comment

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

You can use OCF_PL_NEXT_RET() here, so you can drop the else block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've reverted this part according to Michal's comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

...and done it again with OCF_PL_NEXT_RET ;)

if (error) {
ocf_cache_log(cache, log_err,
"ERROR: Failed to clear superblock\n");
OCF_PL_FINISH_RET(context->pipeline, -OCF_ERR_WRITE_CACHE);
Copy link
Contributor

Choose a reason for hiding this comment

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

Even if zeroing superblock has failed, the detach pipeline shouldn't be interrupted. At this point the metadata has already been deinitialized so zeroing superblock is nice to have but it's not necessary.
We could inform the user what has happened, warn about the remnants of metadata and suggest they remove it on their own, but the detach pipeline should proceed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

src/mngt/ocf_mngt_cache.c Show resolved Hide resolved
@Deixx Deixx force-pushed the detach-zero-superblock branch 3 times, most recently from b1ac551 to 6a81bde Compare February 6, 2025 08:23
Deixx added 2 commits February 6, 2025 09:24
Metadata from a detached cache device is not meant to be loaded.

Signed-off-by: Daniel Madej <[email protected]>
@Deixx Deixx force-pushed the detach-zero-superblock branch from 6a81bde to 3263503 Compare February 6, 2025 08:25
@robertbaldyga robertbaldyga merged commit ba4b81a into Open-CAS:master Feb 6, 2025
2 checks passed
@Deixx Deixx deleted the detach-zero-superblock branch February 6, 2025 11:12
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