osd: return appropriate error if the object is not manifest#45137
osd: return appropriate error if the object is not manifest#45137athanatos merged 1 commit intoceph:masterfrom
Conversation
373f659 to
dae21e8
Compare
|
jenkins test make check |
|
@myoungwon could you please take a look at https://tracker.ceph.com/issues/51627#note-9? |
|
Sure |
src/osd/objclass.cc
Outdated
| ceph_assert(ctx->obc); | ||
| if (!ctx->obc->obs.oi.has_manifest() || !ctx->obc->obs.oi.manifest.is_chunked()) { | ||
| return -ENOLINK; | ||
| } |
There was a problem hiding this comment.
This seems like it should be in the implementation of get_manifest_ref_count?
|
LGTM pending test run |
|
@myoungwon http://pulpito.front.sepia.ceph.com/yuriw-2022-03-18_13:48:12-rados-wip-yuri6-testing-2022-03-17-1547-distro-default-smithi/6745170/ |
|
jenkins test make check |
|
@neha-ojha Ok, I'll check the issue. |
|
The second commit seems to revert the first. Is that intentional? |
|
Yes, I didn't catch a corner case before---the head is not manifest, but its snapshot has manifest. In this case, returning an error (because the head is not manifest) will not calculate reference count correctly. Therefore, I added the second commit---this commit try to search all snapshot the object ha. If this change is acceptable, I'll squash the two commits into one. |
|
AFAICT, this is right. Please update PR description, squash the commits, and add the needs-qa flag since this needs to be retested. |
If a user sends reference_chunk() to the original object (not manifest object) which has not recovered snapshots, the OSD triggers assert() because reference_chunk() try to find adjacent unrecovered clones, resulting in the assert(). This is because the original object does not wait the recovery of snapshots. To avoid this, this commit add a condition to check a base snapshot is readable whether the object is manifest or not. If the base snapshot is valid and the snapshot is manifest , osd try to calculate reference count. fixes: https://tracker.ceph.com/issues/54509 Signed-off-by: Myoungwon Oh <myoungwon.oh@samsung.com>
|
@athanatos this passed tests, pls approve and merge at will |
Unlike existing object, manifest object requires valid adjacent
clones to calculate references.
If a user sends reference_chunk()---this is only for manifest object---
to the original object (not manifest object) which has not recovered snapshots,
the OSD triggers assert() because reference_chunk() try to
find adjacent clones, resulting in the assert(). This is because the original object
does not need to wait the recovery of snapshot.
So, this PR return an error if the object is not manifeset.
fixes: https://tracker.ceph.com/issues/54509
Signed-off-by: Myoungwon Oh myoungwon.oh@samsung.com
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 tox