Skip to content

Commit

Permalink
Change some error handling to results
Browse files Browse the repository at this point in the history
  • Loading branch information
IsaacKhor committed Aug 13, 2024
1 parent 644e08a commit e1220b4
Show file tree
Hide file tree
Showing 17 changed files with 301 additions and 233 deletions.
16 changes: 8 additions & 8 deletions src/backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,22 @@ class backend
public:
virtual ~backend() {}

virtual int write(std::string name, smartiov &iov) = 0;
virtual int read(std::string name, off_t offset, smartiov &iov) = 0;
virtual int delete_obj(std::string name) = 0;
virtual result<int> write(std::string name, smartiov &iov) = 0;
virtual result<int> read(std::string name, off_t offset, smartiov &iov) = 0;
virtual result<void> delete_obj(std::string name) = 0;

virtual request *aio_write(std::string name, smartiov &iov) = 0;
virtual request *aio_read(std::string name, off_t offset,
smartiov &iov) = 0;
virtual request *aio_delete(std::string name) = 0;

int write(std::string name, void *buf, size_t len)
result<int> write(std::string name, void *buf, size_t len)
{
smartiov iov((char *)buf, len);
return write(name, iov);
}

int read(std::string name, off_t offset, void *buf, size_t len)
result<int> read(std::string name, off_t offset, void *buf, size_t len)
{
smartiov iov((char *)buf, len);
return read(name, offset, iov);
Expand All @@ -46,10 +46,10 @@ class backend
return aio_read(name, offset, iov);
}

virtual opt<u64> get_size(std::string name) = 0;
virtual opt<vec<byte>> read_whole_obj(std::string name) = 0;
virtual result<u64> get_size(std::string name) = 0;
virtual result<vec<byte>> read_whole_obj(std::string name) = 0;
virtual bool exists(std::string name) = 0;
};

extern std::shared_ptr<backend> make_rados_backend(rados_ioctx_t io);
rados_ioctx_t connect_to_pool(str pool_name);
result<rados_ioctx_t> connect_to_pool(str pool_name);
31 changes: 16 additions & 15 deletions src/bdev_lsvd.cc
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,8 @@ struct lsvd_bdev_io_channel {
spdk_io_channel *io_channel;
};

int bdev_lsvd_create(str img_name, rados_ioctx_t ioctx, lsvd_config cfg)
auto bdev_lsvd_create(str img_name, rados_ioctx_t ioctx,
lsvd_config cfg) -> result<void>
{
assert(!img_name.empty());

Expand All @@ -189,7 +190,7 @@ int bdev_lsvd_create(str img_name, rados_ioctx_t ioctx, lsvd_config cfg)
img = uptr<lsvd_image>(new lsvd_image(img_name, ioctx, cfg));
} catch (std::runtime_error &e) {
log_error("Failed to create image '{}': {}", img_name, e.what());
return -1;
return outcome::failure(std::errc::io_error);
}

auto iodev = new lsvd_iodevice(std::move(img));
Expand All @@ -213,22 +214,22 @@ int bdev_lsvd_create(str img_name, rados_ioctx_t ioctx, lsvd_config cfg)
log_error("Failed to register bdev: err {}", (err));
spdk_io_device_unregister(
iodev, [](void *ctx) { delete (lsvd_iodevice *)ctx; });
return err;

return std::error_code(err, std::system_category());
}

return 0;
return outcome::success();
}

int bdev_lsvd_create(str pool_name, str image_name, str user_cfg)
auto bdev_lsvd_create(str pool_name, str image_name,
str user_cfg) -> result<void>
{
auto be = connect_to_pool(pool_name);
auto cfg = lsvd_config::from_user_cfg(user_cfg);
PR_RET_IF(!cfg.has_value(), -1, "Failed to read config file");

return bdev_lsvd_create(image_name, be, cfg.value());
auto be = BOOST_OUTCOME_TRYX(connect_to_pool(pool_name));
auto cfg = BOOST_OUTCOME_TRYX(lsvd_config::from_user_cfg(user_cfg));
return bdev_lsvd_create(image_name, be, cfg);
}

void bdev_lsvd_delete(str img_name, std::function<void(int)> cb)
void bdev_lsvd_delete(str img_name, std::function<void(result<void>)> cb)
{
log_info("Deleting image '{}'", img_name);
auto rc = spdk_bdev_unregister_by_name(
Expand All @@ -237,14 +238,14 @@ void bdev_lsvd_delete(str img_name, std::function<void(int)> cb)
// it should work
[](void *arg, int rc) {
log_info("Image deletion done, rc = {}", rc);
auto cb = (std::function<void(int)> *)arg;
(*cb)(rc);
auto cb = (std::function<void(result<void>)> *)arg;
(*cb)(errcode_to_result<void>(rc));
delete cb;
},
new std::function<void(int)>(cb));
new std::function<void(result<void>)>(cb));

if (rc != 0) {
log_error("Failed to delete image '{}': {}", img_name, rc);
cb(rc);
cb(outcome::failure(std::error_code(rc, std::system_category())));
}
}
9 changes: 5 additions & 4 deletions src/bdev_lsvd.h
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
#pragma once

#include <rados/librados.h>
#include <functional>
#include <rados/librados.h>

#include "config.h"
#include "utils.h"

int bdev_lsvd_create(str img_name, rados_ioctx_t io_ctx, lsvd_config cfg);
int bdev_lsvd_create(str pool_name, str image_name, str cfg_path);
void bdev_lsvd_delete(str img_name, std::function<void(int)> cb);
result<void> bdev_lsvd_create(str img_name, rados_ioctx_t io, lsvd_config cfg);
result<void> bdev_lsvd_create(str pool_name, str image_name, str cfg_path);
void bdev_lsvd_delete(str img_name, std::function<void(result<void>)> cb);
12 changes: 6 additions & 6 deletions src/bdev_lsvd_rpc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ static void rpc_bdev_lsvd_create(spdk_jsonrpc_request *req_json,
return;
}

rc = bdev_lsvd_create(req->pool_name, req->image_name, req->config);
if (rc != 0) {
spdk_jsonrpc_send_error_response(req_json, rc,
auto res = bdev_lsvd_create(req->pool_name, req->image_name, req->config);
if (!res.has_value()) {
spdk_jsonrpc_send_error_response(req_json, res.error().value(),
"Failed to create lsvd bdev");
return;
}
Expand Down Expand Up @@ -88,13 +88,13 @@ static void rpc_bdev_lsvd_delete(struct spdk_jsonrpc_request *req_json,
return;
}

bdev_lsvd_delete(req->image_name, [=](int rc) {
if (rc == 0) {
bdev_lsvd_delete(req->image_name, [=](result<void> res) {
if (res.has_value()) {
auto w = spdk_jsonrpc_begin_result(req_json);
spdk_json_write_bool(w, true);
spdk_jsonrpc_end_result(req_json, w);
} else {
log_error("Failed to destroy lsvd bdev, rc = {}", rc);
log_error("Failed to destroy lsvd bdev: {}", res.error().message());
spdk_jsonrpc_send_error_response(req_json, rc,
"Failed to destroy lsvd bdev");
}
Expand Down
51 changes: 26 additions & 25 deletions src/config.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,37 +11,32 @@

lsvd_config lsvd_config::get_default() { return lsvd_config(); }

opt<lsvd_config> lsvd_config::from_user_cfg(str cfg)
result<lsvd_config> lsvd_config::from_user_cfg(str cfg)
{
auto c = get_default();
if (cfg.empty())
return c;

try {
c.parse_file("/usr/local/etc/lsvd.json", true);
c.parse_file("./lsvd.json", true);
BOOST_OUTCOME_TRYX(c.parse_file("/usr/local/etc/lsvd.json", true));
BOOST_OUTCOME_TRYX(c.parse_file("./lsvd.json", true));

if (cfg[0] == '{') {
c.parse_json(cfg);
} else {
c.parse_file(cfg, false);
}
return c;
} catch (const std::exception &e) {
log_error("Failed to parse config '{}': {}", cfg, e.what());
return std::nullopt;
if (cfg[0] == '{') {
BOOST_OUTCOME_TRYX(c.parse_json(cfg));
} else {
BOOST_OUTCOME_TRYX(c.parse_file(cfg, false));
}
return c;
}

// https://stackoverflow.com/questions/116038/how-do-i-read-an-entire-file-into-a-stdstring-in-c
auto read_file(std::string_view path) -> std::string
auto read_file(std::string_view path) -> result<str>
{
constexpr auto read_size = std::size_t(4096);
auto stream = std::ifstream(path.data());
stream.exceptions(std::ios_base::badbit);

if (not stream)
throw std::ios_base::failure("file does not exist");
return outcome::failure(std::errc::no_such_file_or_directory);

auto out = std::string();
auto buf = std::string(read_size, '\0');
Expand All @@ -51,20 +46,24 @@ auto read_file(std::string_view path) -> std::string
return out;
}

void lsvd_config::parse_file(str path, bool can_be_missing)
auto lsvd_config::parse_file(str path, bool allow_missing) -> result<void>
{
assert(!path.empty());
if (access(path.c_str(), F_OK) == -1) {
if (can_be_missing) {
log_info("Optional config file missing: {}, continuing...", path);
return;

auto cfg = read_file(path);
if (cfg.has_error()) {
if (cfg.error() == std::errc::no_such_file_or_directory &&
allow_missing) {
return outcome::success();
} else {
log_error("Failed to read config file '{}': {}", path,
cfg.error().message());
return cfg.as_failure();
}
log_error("Config file not found: {}", path);
return;
}

auto cfg = read_file(path);
parse_json(cfg);
BOOST_OUTCOME_TRYX(parse_json(cfg.value()));
return outcome::success();
}

#define JSON_GET_BOOL(key) \
Expand Down Expand Up @@ -103,7 +102,7 @@ void lsvd_config::parse_file(str path, bool can_be_missing)
} \
} while (0)

void lsvd_config::parse_json(str json)
result<void> lsvd_config::parse_json(str json)
{
auto js = nlohmann::json::parse(json);

Expand All @@ -126,4 +125,6 @@ void lsvd_config::parse_json(str json)
JSON_GET_UINT(gc_threshold_pc);
JSON_GET_UINT(gc_write_window);
JSON_GET_BOOL(no_gc);

return outcome::success();
}
9 changes: 5 additions & 4 deletions src/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,18 @@ class lsvd_config
return fspath(wlog_dir) / filename;
}

static lsvd_config get_default();

/**
* Read LSVD configuration from user-supplied string. The string can be
* either a json string containing the configuration, the path to a file
* containing the same, or empty, in which case it will be ignored.
*/
static opt<lsvd_config> from_user_cfg(str cfg);
static lsvd_config get_default();
static result<lsvd_config> from_user_cfg(str cfg);

private:
lsvd_config() {}

void parse_json(str js);
void parse_file(str path, bool can_be_missing = true);
result<void> parse_json(str js);
result<void> parse_file(str path, bool can_be_missing = true);
};
Loading

0 comments on commit e1220b4

Please sign in to comment.