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

Call recreateSymlinks when not found during Readlink #822

Merged
merged 1 commit into from Feb 19, 2021

Conversation

umohnani8
Copy link
Collaborator

@umohnani8 umohnani8 commented Feb 16, 2021

If the symlinks under the l directory are lost during a
sudden reboot, we can hit an error when grabbing the lower
directories and reading the symlinks before they are recreated
in the Get function. Add a call to recreateSymlinks in this
code path as well.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1921128

Signed-off-by: Urvashi Mohnani umohnani@redhat.com

@umohnani8
Copy link
Collaborator Author

@rhatdan @nalind PTAL

Copy link
Member

@rhatdan rhatdan left a comment

Choose a reason for hiding this comment

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

LGTM

drivers/overlay/overlay.go Outdated Show resolved Hide resolved
If the symlinks under the l directory are lost during a
sudden reboot, we can hit an error when grabbing the lower
directories and reading the symlinks before they are recreated
in the Get function. Add a call to recreateSymlinks in this
code path as well.

Signed-off-by: Urvashi Mohnani <umohnani@redhat.com>
@rhatdan
Copy link
Member

rhatdan commented Feb 17, 2021

LGTM

@rhatdan
Copy link
Member

rhatdan commented Feb 19, 2021

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

LGTM

@vrothberg vrothberg merged commit b393db5 into containers:master Feb 19, 2021
@giuseppe
Copy link
Member

Should we at least warn that the storage is corrupted and files might be missing or are partially stored?

@umohnani8
Copy link
Collaborator Author

@giuseppe there is a warning that we can't find the symlinks so going to restore those https://github.com/containers/storage/pull/822/files#diff-4b0197157e354c66e154f5cdc6fea71c4e43732b4bad32d8652f23bd588e51a1R819

@giuseppe
Copy link
Member

@giuseppe there is a warning that we can't find the symlinks so going to restore those https://github.com/containers/storage/pull/822/files#diff-4b0197157e354c66e154f5cdc6fea71c4e43732b4bad32d8652f23bd588e51a1R819

I am just worried that users will get the impression this would fix the storage corruptions but there could be other issues that we are not able to detect yet, such as missing or incomplete files in the pulled images. Maybe a warning saying the safest is probably to completely wipe the storage if possible to not incur in weird errors?

@vrothberg
Copy link
Member

SGTM

@umohnani8
Copy link
Collaborator Author

Follow up with warning in #833

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.

None yet

5 participants