Skip to content

Commit

Permalink
blk/bluestore: remove BlockDevice::supported_bdev_label()
Browse files Browse the repository at this point in the history
Block device got this interface function with #7145.
In bluestore, bdev label just means "first disk block, located at #0".
For other devices, there is deeper meaning: https://docs.pmem.io/ndctl-user-guide/managing-label-storage-areas-lsa

It is most likely that two distinct features were joined by name.
It is obvious that any block device will be able to write to its first block.

Signed-off-by: Adam Kupczyk <akupczyk@ibm.com>
  • Loading branch information
aclamk committed Apr 26, 2024
1 parent 8e9f5c5 commit f778349
Show file tree
Hide file tree
Showing 7 changed files with 51 additions and 81 deletions.
1 change: 0 additions & 1 deletion src/blk/BlockDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ class BlockDevice {

static BlockDevice *create(
CephContext* cct, const std::string& path, aio_callback_t cb, void *cbpriv, aio_callback_t d_cb, void *d_cbpriv);
virtual bool supported_bdev_label() { return true; }
virtual bool is_rotational() { return rotational; }

// HM-SMR-specific calls
Expand Down
1 change: 0 additions & 1 deletion src/blk/pmem/PMEMDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ class PMEMDevice : public BlockDevice {
public:
PMEMDevice(CephContext *cct, aio_callback_t cb, void *cbpriv);

bool supported_bdev_label() override { return !devdax_device; }
void aio_submit(IOContext *ioc) override;

int collect_metadata(const std::string& prefix, std::map<std::string,std::string> *pm) const override;
Expand Down
2 changes: 0 additions & 2 deletions src/blk/spdk/NVMEDevice.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ class NVMEDevice : public BlockDevice {

NVMEDevice(CephContext* cct, aio_callback_t cb, void *cbpriv);

bool supported_bdev_label() override { return false; }

static bool support(const std::string& path);

void aio_submit(IOContext *ioc) override;
Expand Down
7 changes: 0 additions & 7 deletions src/os/bluestore/BlueFS.cc
Original file line number Diff line number Diff line change
Expand Up @@ -532,13 +532,6 @@ int BlueFS::add_block_device(unsigned id, const string& path, bool trim,
return 0;
}

bool BlueFS::bdev_support_label(unsigned id)
{
ceph_assert(id < bdev.size());
ceph_assert(bdev[id]);
return bdev[id]->supported_bdev_label();
}

uint64_t BlueFS::get_block_device_size(unsigned id) const
{
if (id < bdev.size() && bdev[id])
Expand Down
1 change: 0 additions & 1 deletion src/os/bluestore/BlueFS.h
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,6 @@ class BlueFS {

int add_block_device(unsigned bdev, const std::string& path, bool trim,
bluefs_shared_alloc_context_t* _shared_alloc = nullptr);
bool bdev_support_label(unsigned id);
uint64_t get_block_device_size(unsigned bdev) const;

// handler for discard event
Expand Down
118 changes: 50 additions & 68 deletions src/os/bluestore/BlueStore.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6636,11 +6636,9 @@ int BlueStore::_open_bdev(bool create)
bdev->try_discard(whole_device, false);
}

if (bdev->supported_bdev_label()) {
r = _check_or_set_bdev_label(p, bdev->get_size(), "main", create);
if (r < 0)
goto fail_close;
}
r = _check_or_set_bdev_label(p, bdev->get_size(), "main", create);
if (r < 0)
goto fail_close;

// initialize global block parameters
block_size = bdev->get_block_size();
Expand Down Expand Up @@ -7162,17 +7160,15 @@ int BlueStore::_minimal_open_bluefs(bool create)
goto free_bluefs;
}

if (bluefs->bdev_support_label(BlueFS::BDEV_DB)) {
r = _check_or_set_bdev_label(
bfn,
bluefs->get_block_device_size(BlueFS::BDEV_DB),
"bluefs db", create);
if (r < 0) {
derr << __func__
<< " check block device(" << bfn << ") label returned: "
<< cpp_strerror(r) << dendl;
goto free_bluefs;
}
r = _check_or_set_bdev_label(
bfn,
bluefs->get_block_device_size(BlueFS::BDEV_DB),
"bluefs db", create);
if (r < 0) {
derr << __func__
<< " check block device(" << bfn << ") label returned: "
<< cpp_strerror(r) << dendl;
goto free_bluefs;
}
bluefs_layout.shared_bdev = BlueFS::BDEV_SLOW;
bluefs_layout.dedicated_db = true;
Expand Down Expand Up @@ -7209,16 +7205,14 @@ int BlueStore::_minimal_open_bluefs(bool create)
goto free_bluefs;
}

if (bluefs->bdev_support_label(BlueFS::BDEV_WAL)) {
r = _check_or_set_bdev_label(
bfn,
bluefs->get_block_device_size(BlueFS::BDEV_WAL),
"bluefs wal", create);
if (r < 0) {
derr << __func__ << " check block device(" << bfn
<< ") label returned: " << cpp_strerror(r) << dendl;
goto free_bluefs;
}
r = _check_or_set_bdev_label(
bfn,
bluefs->get_block_device_size(BlueFS::BDEV_WAL),
"bluefs wal", create);
if (r < 0) {
derr << __func__ << " check block device(" << bfn
<< ") label returned: " << cpp_strerror(r) << dendl;
goto free_bluefs;
}

bluefs_layout.dedicated_wal = true;
Expand Down Expand Up @@ -8295,14 +8289,12 @@ int BlueStore::add_new_bluefs_device(int id, const string& dev_path)
cct->_conf->bdev_enable_discard);
ceph_assert(r == 0);

if (bluefs->bdev_support_label(BlueFS::BDEV_NEWWAL)) {
r = _check_or_set_bdev_label(
p,
bluefs->get_block_device_size(BlueFS::BDEV_NEWWAL),
"bluefs wal",
true);
ceph_assert(r == 0);
}
r = _check_or_set_bdev_label(
p,
bluefs->get_block_device_size(BlueFS::BDEV_NEWWAL),
"bluefs wal",
true);
ceph_assert(r == 0);

bluefs_layout.dedicated_wal = true;
} else if (id == BlueFS::BDEV_NEWDB) {
Expand All @@ -8316,14 +8308,12 @@ int BlueStore::add_new_bluefs_device(int id, const string& dev_path)
cct->_conf->bdev_enable_discard);
ceph_assert(r == 0);

if (bluefs->bdev_support_label(BlueFS::BDEV_NEWDB)) {
r = _check_or_set_bdev_label(
p,
bluefs->get_block_device_size(BlueFS::BDEV_NEWDB),
"bluefs db",
true);
ceph_assert(r == 0);
}
r = _check_or_set_bdev_label(
p,
bluefs->get_block_device_size(BlueFS::BDEV_NEWDB),
"bluefs db",
true);
ceph_assert(r == 0);
bluefs_layout.shared_bdev = BlueFS::BDEV_SLOW;
bluefs_layout.dedicated_db = true;
}
Expand Down Expand Up @@ -8445,14 +8435,12 @@ int BlueStore::migrate_to_new_bluefs_device(const set<int>& devs_source,
cct->_conf->bdev_enable_discard);
ceph_assert(r == 0);

if (bluefs->bdev_support_label(BlueFS::BDEV_NEWWAL)) {
r = _check_or_set_bdev_label(
dev_path,
bluefs->get_block_device_size(BlueFS::BDEV_NEWWAL),
"bluefs wal",
true);
ceph_assert(r == 0);
}
r = _check_or_set_bdev_label(
dev_path,
bluefs->get_block_device_size(BlueFS::BDEV_NEWWAL),
"bluefs wal",
true);
ceph_assert(r == 0);
} else if (id == BlueFS::BDEV_NEWDB) {
target_name = "block.db";
target_size = cct->_conf->bluestore_block_db_size;
Expand All @@ -8463,14 +8451,12 @@ int BlueStore::migrate_to_new_bluefs_device(const set<int>& devs_source,
cct->_conf->bdev_enable_discard);
ceph_assert(r == 0);

if (bluefs->bdev_support_label(BlueFS::BDEV_NEWDB)) {
r = _check_or_set_bdev_label(
dev_path,
bluefs->get_block_device_size(BlueFS::BDEV_NEWDB),
"bluefs db",
true);
ceph_assert(r == 0);
}
r = _check_or_set_bdev_label(
dev_path,
bluefs->get_block_device_size(BlueFS::BDEV_NEWDB),
"bluefs db",
true);
ceph_assert(r == 0);
}

bluefs->umount();
Expand Down Expand Up @@ -8568,12 +8554,10 @@ int BlueStore::expand_devices(ostream& out)
<<": can't find device path " << dendl;
continue;
}
if (bluefs->bdev_support_label(devid)) {
if (_set_bdev_label_size(p, size) >= 0) {
out << devid
if (_set_bdev_label_size(p, size) >= 0) {
out << devid
<< " : size label updated to " << size
<< std::endl;
}
}
}
uint64_t size0 = fm->get_size();
Expand All @@ -8583,12 +8567,10 @@ int BlueStore::expand_devices(ostream& out)
<< " : expanding " << " from 0x" << std::hex
<< size0 << " to 0x" << size << std::dec << std::endl;
_write_out_fm_meta(size);
if (bdev->supported_bdev_label()) {
if (_set_bdev_label_size(path, size) >= 0) {
out << bluefs_layout.shared_bdev
<< " : size label updated to " << size
<< std::endl;
}
if (_set_bdev_label_size(path, size) >= 0) {
out << bluefs_layout.shared_bdev
<< " : size label updated to " << size
<< std::endl;
}
_close_db_and_around();

Expand Down

0 comments on commit f778349

Please sign in to comment.