mon: block osd pool mksnap for fs pools#51275
Conversation
|
jenkins test make check |
1 similar comment
|
jenkins test make check |
|
@mchangir Needs a bit more explanation in the commit message. |
0f6fd5f to
109f9c9
Compare
ack; added a detailed note in the commit message |
|
LGTM. Need @rzarzynski review though. |
|
This PR is basically a follow-up to #47753. Linking for the sake of clarity. |
| if (pp->snap_exists(snapname.c_str())) { | ||
| ss << "pool " << poolstr << " snap " << snapname << " already exists"; | ||
| } else { | ||
| if (const auto& fsmap = mon.mdsmon()->get_fsmap(); fsmap.pool_in_use(pool)) { |
There was a problem hiding this comment.
From the commit description:
Pool level snaps and mon managed snaps can't co-exist (...)
Isn't this a thinko? Per #47753 (comment) I started thinking it's about Pool level snaps and FS level snaps.
There was a problem hiding this comment.
yes; this is about FS level snaps and Pool level snaps
its just the nomenclature that @gregsfortytwo has used to refer to it (AFAIUI)
but looks like the phrasing should be changed to FS level snaps and Pool level snaps can't coexist
There was a problem hiding this comment.
phrasing changed to "Pool-level and fs-level snapshots ..."
| base_cmd = f'osd pool mksnap {test_pool_name} snap3' | ||
| self.run_cluster_cmd(base_cmd) | ||
|
|
||
| with self.assertRaises(CommandFailedError): |
Commit 23db15d disabled pool snaps for the rados mksnap path. But ceph osd pool mksnap was an alternate way that pool snaps could be created. This commit disables pool snaps via this alternate path as well. NOTE: Pool-level snaps and fs-level snaps can't co-exist since snap IDs are likely to clash between the two different mechanisms and can result in unintentional data loss when either of the snaps are deleted. Fixes: https://tracker.ceph.com/issues/59552 Signed-off-by: Milind Changire <mchangir@redhat.com>
Signed-off-by: Milind Changire <mchangir@redhat.com>
109f9c9 to
ab64bfa
Compare
|
rebased as well ... |
|
jenkins retest this please |
|
jenkins test windows |
|
@yuriw please pick this up for main branch test runs. |
|
@yuriw: it's ready for testing. |
Fixes: https://tracker.ceph.com/issues/59552
Signed-off-by: Milind Changire mchangir@redhat.com
Contribution Guidelines
To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "pacific"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard cephadmjenkins test apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume toxjenkins test windows