blk/zoned: Add reset_zones functionality for zoned devices #41076
Conversation
|
Values before reset |
src/blk/zoned/HMSMRDevice.h
Outdated
| @@ -41,6 +41,7 @@ class HMSMRDevice final : public BlockDevice { | |||
| string vdo_name; | |||
|
|
|||
| std::string devname; ///< kernel dev name (/sys/block/$devname), if any | |||
| int zbd_file_descriptor; /// zbd_open File Descriptor | |||
There was a problem hiding this comment.
I'd say call this just zbd_dev.
There was a problem hiding this comment.
Updated
Thank you.
src/blk/zoned/HMSMRDevice.cc
Outdated
| @@ -412,6 +412,14 @@ void HMSMRDevice::_detect_vdo() | |||
| return; | |||
| } | |||
|
|
|||
| // Will reset the write pointer of all zones from start to end(both inclusive) | |||
| bool HMSMRDevice::reset_zones(uint64_t zone_num_range_start, uint64_t zone_num_range_end) { | |||
There was a problem hiding this comment.
Return int to be consistent with the remaining functions.
Nit: Omit range from parameter names.
There was a problem hiding this comment.
Updated.
Thank you.
src/blk/zoned/HMSMRDevice.cc
Outdated
| ceph_assert(is_smr()); | ||
| uint64_t len = (zone_num_range_end + 1 - zone_num_range_start) * zone_size; | ||
| ceph_assert(len > 0); | ||
| return !zbd_reset_zones(zbd_file_descriptor, zone_num_range_start * zone_size, len); |
There was a problem hiding this comment.
In case of failure it would be useful to print the error message.
| @@ -200,6 +200,11 @@ class BlockDevice { | |||
| return conventional_region_size; | |||
| } | |||
|
|
|||
| virtual bool reset_zones(uint64_t zone_num_range_start, uint64_t zone_num_range_end) { | |||
| ceph_assert(is_smr()); | |||
There was a problem hiding this comment.
Not sure if we should assert here given that right below we return false. Let's see what Ceph folks say.
Rishabh4275
force-pushed
the
zoned-cleaner-zbd-reset
branch
from
be140da
to
11ecf21
Compare
Signed-off-by: Rishabh Chawla <rishabhchawla1995@gmail.com>
Rishabh4275
force-pushed
the
zoned-cleaner-zbd-reset
branch
from
11ecf21
to
b493226
Compare
|
@tchaikov can you please assign this to a reviewer? Thanks. |
| @@ -200,6 +200,11 @@ class BlockDevice { | |||
| return conventional_region_size; | |||
| } | |||
|
|
|||
| virtual uint64_t reset_zones(uint64_t zone_num_start, uint64_t zone_num_end) { | |||
There was a problem hiding this comment.
is this method called anywhere?
There was a problem hiding this comment.
Not yet. It will be called in the next PR by the code that cleans zones.
There was a problem hiding this comment.
i'd like to see how it is used for better understanding of this change. IMHO, it'd be better to have the change which introduces the consumer in the same PR.
|
Shifting to #40923 |
|
This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
|
This pull request has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs for another 30 days. |
|
This pull request has been automatically closed because there has been no activity for 90 days. Please feel free to reopen this pull request (or open a new one) if the proposed change is still appropriate. Thank you for your contribution! |
Signed-off-by: Rishabh Chawla rishabhchawla1995@gmail.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 apijenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox