os/bluestore: fix min_alloc_size at mkfs time #13192
Conversation
src/os/bluestore/BlueStore.cc
Outdated
| try { | ||
| ::decode(ondisk_format, p); | ||
| } catch (buffer::error& e) { | ||
| } |
There was a problem hiding this comment.
dump some error on exception as we suppose they are possible?
src/os/bluestore/BlueStore.cc
Outdated
| } | ||
| bl.clear(); | ||
| { | ||
| r = db->get(PREFIX_SUPER, "compat_ondisk_format", &bl); |
There was a problem hiding this comment.
suggest do at least assert(r>=0) afterwards
src/os/bluestore/BlueStore.cc
Outdated
| } | ||
| } | ||
|
|
||
| _set_alloc_sizes(); |
There was a problem hiding this comment.
Perhaps it's better to be inserted in the next commit
There was a problem hiding this comment.
And currently is might be called without prior min_alloc_size var setting.
src/os/bluestore/BlueStore.cc
Outdated
| // - super: added min_readable_ondisk_format | ||
| // - super: added min_compat_ondisk_format | ||
| ondisk_format = 2; | ||
| _prepare_ondisk_format_super(t); |
There was a problem hiding this comment.
this's fixed in the next commit but makes this one broken - hence might produce issues with bisect or cherry-pick...
| derr << __func__ << " failed to read min_min_alloc_size" << dendl; | ||
| return -EIO; | ||
| } | ||
| t->set(PREFIX_SUPER, "min_alloc_size", bl); |
There was a problem hiding this comment.
Don't see any read of that parameter, looks like min_alloc_size in uninitialized if mkfs is bypassed.
Signed-off-by: Sage Weil <sage@redhat.com>
Note the version of the ondisk format and the oldest version that is allowed to read us. Signed-off-by: Sage Weil <sage@redhat.com>
It is an ongoing challenge to allow min_alloc_size to be varied on an existing bluestore instance, and the code paths are not well tested. Avoid the complexity entirely by fixing min_alloc_size at mkfs time. Signed-off-by: Sage Weil <sage@redhat.com>
liewegas
force-pushed
the
wip-bluestore-min-alloc-size
branch
from
7920b76
to
9c9a558
Compare
|
fixed, cleaned up, tested trivial upgrade w/ vstart |
src/os/bluestore/BlueStore.cc
Outdated
| db->get(PREFIX_SUPER, "min_min_alloc_size", &bl); | ||
| auto p = bl.begin(); | ||
| try { | ||
| ::decode(min_alloc_size, p); |
There was a problem hiding this comment.
Suggest to decode using a standalone variable instead of min_alloc_size. With an explicit type specification for that var (equal to min_min_alloc_size). Otherwise one might have a well hidden issue when changing min_alloc_size type in future.
Defend against future changes to min_alloc_size. Signed-off-by: Sage Weil <sage@redhat.com>
|
retest this please |
No description provided.