-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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
reef: librados: make querying pools for selfmanaged snaps reliable #55026
Conversation
This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved |
c1d2016
to
b3483a5
Compare
This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved |
If get_pool_is_selfmanaged_snaps_mode() is invoked on a fresh RADOS client instance that still lacks an osdmap, it returns false, same as for "this pool is not in selfmanaged snaps mode". The same happens if the pool in question doesn't exist since the signature doesn't allow to return an error. The motivation for this API was to prevent users from running "rados cppool" on a pool with unmanaged snapshots and deleting the original thinking that they have a full copy. Unfortunately, it's exactly "rados cppool" that fell into this trap, so no warning is printed and --yes-i-really-mean-it flag isn't enforced. Fixes: https://tracker.ceph.com/issues/63607 Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 0999e63) Conflicts: PendingReleaseNotes [ moved to >=18.2.2 section ]
Safeguards in rados CLI tool isn't really the subject of this test, but it fits nicely. Signed-off-by: Ilya Dryomov <idryomov@gmail.com> (cherry picked from commit 2b135a2)
b3483a5
to
af5fe53
Compare
Rebased to resolve a conflict in |
jenkins test make check |
|
||
/// check if pool has or had selfmanaged snaps | ||
bool get_pool_is_selfmanaged_snaps_mode(const std::string& poolname) | ||
__attribute__ ((deprecated)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume it's fine to deprecate in a minor release.
backport tracker: https://tracker.ceph.com/issues/63900
backport of #54644
parent tracker: https://tracker.ceph.com/issues/63607