From e1220b4bff4878e8d50557005fb69ccecb425f67 Mon Sep 17 00:00:00 2001 From: Isaac Khor Date: Tue, 13 Aug 2024 13:29:59 +0000 Subject: [PATCH] Change some error handling to results --- src/backend.h | 16 +++--- src/bdev_lsvd.cc | 31 +++++------ src/bdev_lsvd.h | 9 ++-- src/bdev_lsvd_rpc.cc | 12 ++--- src/config.cc | 51 +++++++++--------- src/config.h | 9 ++-- src/image.cc | 83 +++++++++++++++--------------- src/image.h | 15 +++--- src/imgtool.cc | 18 +++---- src/liblsvd.cc | 11 ++-- src/objects.cc | 57 ++++++++++---------- src/objects.h | 8 +-- src/rados_backend.cc | 70 ++++++++++++------------- src/spdk_frontend.cc | 5 +- src/spdk_wrap.h | 1 + src/utils.h | 120 +++++++++++++++++++++++++++++++++++++------ test/test-seq.cc | 18 ++----- 17 files changed, 301 insertions(+), 233 deletions(-) diff --git a/src/backend.h b/src/backend.h index 5a6f65d1..a33c0a68 100644 --- a/src/backend.h +++ b/src/backend.h @@ -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 write(std::string name, smartiov &iov) = 0; + virtual result read(std::string name, off_t offset, smartiov &iov) = 0; + virtual result 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 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 read(std::string name, off_t offset, void *buf, size_t len) { smartiov iov((char *)buf, len); return read(name, offset, iov); @@ -46,10 +46,10 @@ class backend return aio_read(name, offset, iov); } - virtual opt get_size(std::string name) = 0; - virtual opt> read_whole_obj(std::string name) = 0; + virtual result get_size(std::string name) = 0; + virtual result> read_whole_obj(std::string name) = 0; virtual bool exists(std::string name) = 0; }; extern std::shared_ptr make_rados_backend(rados_ioctx_t io); -rados_ioctx_t connect_to_pool(str pool_name); +result connect_to_pool(str pool_name); diff --git a/src/bdev_lsvd.cc b/src/bdev_lsvd.cc index 661f7da1..92e53a1b 100644 --- a/src/bdev_lsvd.cc +++ b/src/bdev_lsvd.cc @@ -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 { assert(!img_name.empty()); @@ -189,7 +190,7 @@ int bdev_lsvd_create(str img_name, rados_ioctx_t ioctx, lsvd_config cfg) img = uptr(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)); @@ -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 { - 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 cb) +void bdev_lsvd_delete(str img_name, std::function)> cb) { log_info("Deleting image '{}'", img_name); auto rc = spdk_bdev_unregister_by_name( @@ -237,14 +238,14 @@ void bdev_lsvd_delete(str img_name, std::function cb) // it should work [](void *arg, int rc) { log_info("Image deletion done, rc = {}", rc); - auto cb = (std::function *)arg; - (*cb)(rc); + auto cb = (std::function)> *)arg; + (*cb)(errcode_to_result(rc)); delete cb; }, - new std::function(cb)); + new std::function)>(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()))); } } diff --git a/src/bdev_lsvd.h b/src/bdev_lsvd.h index 3028494e..e07e450d 100644 --- a/src/bdev_lsvd.h +++ b/src/bdev_lsvd.h @@ -1,10 +1,11 @@ #pragma once -#include #include +#include #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 cb); +result bdev_lsvd_create(str img_name, rados_ioctx_t io, lsvd_config cfg); +result bdev_lsvd_create(str pool_name, str image_name, str cfg_path); +void bdev_lsvd_delete(str img_name, std::function)> cb); diff --git a/src/bdev_lsvd_rpc.cc b/src/bdev_lsvd_rpc.cc index f9c80e67..bec286e6 100644 --- a/src/bdev_lsvd_rpc.cc +++ b/src/bdev_lsvd_rpc.cc @@ -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; } @@ -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 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"); } diff --git a/src/config.cc b/src/config.cc index 8e8cac04..ca4ed3a8 100644 --- a/src/config.cc +++ b/src/config.cc @@ -11,37 +11,32 @@ lsvd_config lsvd_config::get_default() { return lsvd_config(); } -opt lsvd_config::from_user_cfg(str cfg) +result 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 { 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'); @@ -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 { 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) \ @@ -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 lsvd_config::parse_json(str json) { auto js = nlohmann::json::parse(json); @@ -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(); } diff --git a/src/config.h b/src/config.h index 03b5efb3..3e8636ac 100644 --- a/src/config.h +++ b/src/config.h @@ -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 from_user_cfg(str cfg); - static lsvd_config get_default(); + static result from_user_cfg(str cfg); private: lsvd_config() {} - void parse_json(str js); - void parse_file(str path, bool can_be_missing = true); + result parse_json(str js); + result parse_file(str path, bool can_be_missing = true); }; diff --git a/src/image.cc b/src/image.cc index b92097ec..528dd1f7 100644 --- a/src/image.cc +++ b/src/image.cc @@ -1,9 +1,9 @@ #include -#include #include #include #include #include +#include #include "backend.h" #include "image.h" @@ -15,17 +15,17 @@ const int block_sectors = CACHE_CHUNK_SIZE / 512; -lsvd_image::lsvd_image(std::string name, rados_ioctx_t io, lsvd_config cfg_) +lsvd_image::lsvd_image(str name, rados_ioctx_t io, lsvd_config cfg_) : imgname(name), cfg(cfg_), io(io) { objstore = make_rados_backend(io); rcache = get_read_cache_instance(cfg.rcache_dir, cfg.rcache_bytes, objstore); - read_superblock(); + read_superblock().value(); debug("Found checkpoints: {}", checkpoints); if (checkpoints.size() > 0) - read_from_checkpoint(checkpoints.back()); + read_from_checkpoint(checkpoints.back()).value(); // Roll forward on the log auto last_data_seq = roll_forward_from_last_checkpoint(); @@ -56,22 +56,20 @@ lsvd_image::~lsvd_image() log_info("Image '{}' closed", imgname); } -bool lsvd_image::apply_log(seqnum_t seq) +result lsvd_image::apply_log(seqnum_t seq) { object_reader parser(objstore); - // TODO - auto data_hdr = parser.read_data_hdr(oname(imgname, seq)); - if (!data_hdr.has_value()) - return false; + auto data_hdr = + BOOST_OUTCOME_TRYX(parser.read_data_hdr(oname(imgname, seq))); trace("Recovering log with object at seq {}", seq); - auto ohdr = data_hdr->hdr; + auto ohdr = data_hdr.hdr; if (ohdr->type == OBJ_CHECKPOINT) { log_warn("CORRUPTION: Found checkpoint at seq {} that was not " "present in the superblock.", seq); checkpoints.push_back(seq); - return true; + return outcome::success(); } obj_info[seq] = (data_obj_info){ @@ -83,7 +81,7 @@ bool lsvd_image::apply_log(seqnum_t seq) // Consume log records sector_t offset = 0; vec deleted; - for (auto dmap : data_hdr->data_map) { + for (auto dmap : data_hdr.data_map) { // Update the extent map extmap::obj_offset oo = {seq, offset + ohdr->hdr_sectors}; objmap.update(dmap->lba, dmap->lba + dmap->len, oo, &deleted); @@ -97,22 +95,21 @@ bool lsvd_image::apply_log(seqnum_t seq) THROW_MSG_ON(obj_info[ptr.obj].live >= 0, "Negative live sectors."); } - return true; + return outcome::success(); } -void lsvd_image::read_superblock() +result lsvd_image::read_superblock() { object_reader parser(objstore); - auto superblock = parser.read_superblock(imgname); - THROW_MSG_ON(!superblock, "Failed to read superblock"); + auto superblock = BOOST_OUTCOME_TRYX(parser.read_superblock(imgname)); - size = superblock->vol_size; - uuid_copy(uuid, superblock->uuid); + size = superblock.vol_size; + uuid_copy(uuid, superblock.uuid); - for (auto ckpt : superblock->ckpts) + for (auto ckpt : superblock.ckpts) checkpoints.push_back(ckpt); - for (auto ci : superblock->clones) { + for (auto ci : superblock.clones) { clone_base c; c.name = std::string(ci->name, ci->name_len); c.last_seq = ci->last_seq; @@ -121,15 +118,17 @@ void lsvd_image::read_superblock() debug("Using base image {} upto seq {}", c.name, c.last_seq); clones.push_back(c); } + + return outcome::success(); } -void lsvd_image::read_from_checkpoint(seqnum_t seq) +result lsvd_image::read_from_checkpoint(seqnum_t seq) { object_reader parser(objstore); - auto parsed = parser.read_checkpoint(oname(imgname, seq)); - THROW_MSG_ON(!parsed, "Failed to read checkpoint"); + auto parsed = + BOOST_OUTCOME_TRYX(parser.read_checkpoint(oname(imgname, seq))); - for (auto obj : parsed->objects) { + for (auto obj : parsed.objects) { obj_info[obj->seq] = (data_obj_info){ .hdr = obj->hdr_sectors, .data = obj->data_sectors, @@ -137,10 +136,12 @@ void lsvd_image::read_from_checkpoint(seqnum_t seq) }; } - for (auto m : parsed->dmap) { + for (auto m : parsed.dmap) { extmap::obj_offset oo = {m->obj, m->offset}; objmap.update(m->lba, m->lba + m->len, oo); } + + return outcome::success(); } // Returns last processed object's seqnum @@ -565,7 +566,7 @@ request *lsvd_image::flush(std::function cb) return new flush_request(this, cb); } -int lsvd_image::create_new(std::string name, usize size, rados_ioctx_t io) +result lsvd_image::create_new(str name, usize size, rados_ioctx_t io) { auto be = make_rados_backend(io); auto parser = object_reader(be); @@ -578,46 +579,42 @@ int lsvd_image::create_new(std::string name, usize size, rados_ioctx_t io) vec clones; serialise_superblock(buf, ckpts, clones, uuid, size); - return be->write(name, buf.data(), buf.size()); + auto rc = BOOST_OUTCOME_TRYX(be->write(name, buf.data(), buf.size())); + PR_RET_IF(std::cmp_not_equal(rc, buf.size()), LsvdError::MissingData, + "Failed to write superblock '{}'", name); + return outcome::success(); } -int lsvd_image::get_uuid(str name, uuid_t &uuid, rados_ioctx_t io) +result lsvd_image::get_uuid(str name, uuid_t &uuid, rados_ioctx_t io) { auto be = make_rados_backend(io); auto parser = object_reader(be); - auto osb = parser.read_superblock(name); - PR_RET_IF(!osb, -EEXIST, "Could not read superblock '{}'", name); - - uuid_copy(uuid, osb->uuid); - return 0; + auto osb = BOOST_OUTCOME_TRYX(parser.read_superblock(name)); + uuid_copy(uuid, osb.uuid); + return outcome::success(); } -int lsvd_image::delete_image(std::string name, rados_ioctx_t io) +result lsvd_image::delete_image(str name, rados_ioctx_t io) { auto be = make_rados_backend(io); auto parser = object_reader(be); - auto osb = parser.read_superblock(name); - PR_RET_IF(!osb, -EEXIST, "Could not read superblock '{}'", name); - auto sb = *osb; + auto sb = BOOST_OUTCOME_TRYX(parser.read_superblock(name)); seqnum_t seq; for (auto ckpt : sb.ckpts) { - auto rc = be->delete_obj(oname(name, ckpt)); - PR_RET_IF(rc < 0, rc, "Failed to delete checkpoint '{}'", ckpt); + BOOST_OUTCOME_TRYX(be->delete_obj(oname(name, ckpt))); seq = ckpt; } for (int n = 0; n < 16; seq++, n++) - if (be->delete_obj(oname(name, seq)) >= 0) + if (be->delete_obj(oname(name, seq)).has_value()) n = 0; // delete the superblock last so we can recover from partial deletion return be->delete_obj(name); } -int lsvd_image::clone_image(std::string oldname, std::string newname, - rados_ioctx_t io) +result lsvd_image::clone_image(str oldname, str newname, rados_ioctx_t io) { UNIMPLEMENTED(); - return -1; } \ No newline at end of file diff --git a/src/image.h b/src/image.h index 7a1f0a9e..c0b7fe19 100644 --- a/src/image.h +++ b/src/image.h @@ -41,9 +41,9 @@ class lsvd_image lsvd_image operator=(const lsvd_image &&) = delete; // Log recovery - void read_superblock(); - void read_from_checkpoint(seqnum_t ckpt_id); - bool apply_log(seqnum_t seq); + result read_superblock(); + result read_from_checkpoint(seqnum_t ckpt_id); + result apply_log(seqnum_t seq); seqnum_t roll_forward_from_last_checkpoint(); void recover_from_wlog(); @@ -101,11 +101,10 @@ class lsvd_image // Image management // They all return 0 on success, -errno on failure - static int create_new(std::string name, usize size, rados_ioctx_t io); - static int get_uuid(std::string name, uuid_t &uuid, rados_ioctx_t io); - static int delete_image(std::string name, rados_ioctx_t io); - static int clone_image(std::string oldname, std::string newname, - rados_ioctx_t io); + static result create_new(str name, usize size, rados_ioctx_t io); + static result get_uuid(str name, uuid_t &uuid, rados_ioctx_t io); + static result delete_image(str name, rados_ioctx_t io); + static result clone_image(str oldname, str newname, rados_ioctx_t io); private: void handle_reads(size_t offset, smartiov iovs, vec &requests); diff --git a/src/imgtool.cc b/src/imgtool.cc index 00c030df..febb6ce5 100644 --- a/src/imgtool.cc +++ b/src/imgtool.cc @@ -33,20 +33,17 @@ static usize parseint(str i) static void create(rados_ioctx_t io, str name, usize size, bool thick) { - auto rc = lsvd_image::create_new(name, size, io); - THROW_MSG_ON(rc != 0, "Failed to create new image '{}'", name); + lsvd_image::create_new(name, size, io).value(); } static void remove(rados_ioctx_t io, str name) { - auto rc = lsvd_image::delete_image(name, io); - THROW_MSG_ON(rc != 0, "Failed to delete image '{}'", name); + lsvd_image::delete_image(name, io).value(); } static void clone(rados_ioctx_t io, str src, str dst) { - auto rc = lsvd_image::clone_image(src, dst, io); - THROW_MSG_ON(rc != 0, "Failed to clone image '{}' to '{}'", src, dst); + lsvd_image::clone_image(src, dst, io).value(); } static void info(rados_ioctx_t io, str name) @@ -54,10 +51,8 @@ static void info(rados_ioctx_t io, str name) auto be = make_rados_backend(io); auto parser = object_reader(be); - auto sb = parser.read_superblock(name); - THROW_MSG_ON(!sb, "Superblock not found"); - - auto i = *sb; + auto sb = parser.read_superblock(name).value(); + auto i = sb; char uuid_str[37]; uuid_unparse_lower(i.uuid, uuid_str); @@ -121,8 +116,7 @@ int main(int argc, char **argv) auto pool = vm["pool"].as(); auto img = vm["img"].as(); - auto io = connect_to_pool(pool); - THROW_MSG_ON(io == nullptr, "Failed to connect to pool '{}'", pool); + auto io = connect_to_pool(pool).value(); if (cmd == "create") { auto size = parseint(vm["size"].as()); diff --git a/src/liblsvd.cc b/src/liblsvd.cc index 6c9bd2c1..1c523128 100644 --- a/src/liblsvd.cc +++ b/src/liblsvd.cc @@ -7,8 +7,6 @@ #include #include -#include "backend.h" -#include "config.h" #include "fake_rbd.h" #include "image.h" #include "lsvd_debug.h" @@ -226,13 +224,15 @@ std::pair split_string(std::string s, extern "C" int rbd_create(rados_ioctx_t io, const char *name, uint64_t size, int *order) { - return lsvd_image::create_new(name, size, io); + auto rc = lsvd_image::create_new(name, size, io); + return result_to_rc(rc); } extern "C" int rbd_clone(rados_ioctx_t io, const char *source_img, const char *dest_img) { - return lsvd_image::clone_image(source_img, dest_img, io); + auto rc = lsvd_image::clone_image(source_img, dest_img, io); + return result_to_rc(rc); } /* remove all objects and cache file. @@ -242,7 +242,8 @@ extern "C" int rbd_clone(rados_ioctx_t io, const char *source_img, */ extern "C" int rbd_remove(rados_ioctx_t io, const char *name) { - return lsvd_image::delete_image(name, io); + auto rc = lsvd_image::delete_image(name, io); + return result_to_rc(rc); } extern "C" void rbd_uuid(rbd_image_t image, uuid_t *uuid) diff --git a/src/objects.cc b/src/objects.cc index 98b80c28..3d915c54 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -6,6 +6,7 @@ #include "lsvd_types.h" #include "objects.h" +#include "src/backend.h" #include "utils.h" void serialise_common_hdr(vec &buf, obj_type t, seqnum_t s, u32 hdr, @@ -91,18 +92,18 @@ void serialise_superblock(vec &buf, vec &checkpoints, hdrp->snaps_len = 0; } -opt> object_reader::fetch_object_header(std::string objname) +result> object_reader::fetch_object_header(std::string objname) { vec buf(4096); - auto err = objstore->read(objname, 0, buf.data(), 4096); - RET_IF(err == -ENOENT, std::nullopt); + auto err = BOOST_OUTCOME_TRYX(objstore->read(objname, 0, buf.data(), 4096)); THROW_ERRNO_ON(err < 0, -err, "Failed to read object '{}' header", objname); THROW_MSG_ON(err < 512, "Short read {}/512 on obj '{}'", err, objname); auto h = (common_obj_hdr *)buf.data(); // Validate magic - PR_RET_IF(h->magic != LSVD_MAGIC || h->version != 1, std::nullopt, + PR_RET_IF(h->magic != LSVD_MAGIC || h->version != 1, + LsvdError::InvalidMagic, "Invalid magic or version in object '{}'", objname); if (h->hdr_sectors <= 8) @@ -111,9 +112,9 @@ opt> object_reader::fetch_object_header(std::string objname) // Header is longer than 4096, we have to fetch the rest auto len = h->hdr_sectors * 512; buf.reserve(len); - err = objstore->read(objname, 0, buf.data(), len); - PR_ERR_RET_IF(std::cmp_not_equal(err, len), std::nullopt, err, - "Failed to read object '{}' header", objname); + auto r = BOOST_OUTCOME_TRYX(objstore->read(objname, 0, buf.data(), len)); + PR_RET_IF(std::cmp_not_equal(r, len), LsvdError::MissingData, + "Failed to read object '{}' header", objname); return buf; } @@ -165,19 +166,18 @@ vec deserialise_ptrs_with_len(byte *buf, usize offset, usize len) return ret; } -opt object_reader::read_superblock(std::string oname) +result object_reader::read_superblock(std::string oname) { - auto buf = objstore->read_whole_obj(oname); - PASSTHRU_NULLOPT(buf); - auto hdr = (common_obj_hdr *)buf->data(); - auto hbuf = buf->data(); + auto buf = BOOST_OUTCOME_TRYX(objstore->read_whole_obj(oname)); + auto hdr = (common_obj_hdr *)buf.data(); + auto hbuf = buf.data(); - PR_RET_IF(hdr->magic != LSVD_MAGIC, std::nullopt, + PR_RET_IF(hdr->magic != LSVD_MAGIC, LsvdError::InvalidMagic, "Corrupt object; invalid magic at '{}', found {:x}", oname, hdr->magic); - PR_RET_IF(hdr->version != 1, std::nullopt, + PR_RET_IF(hdr->version != 1, LsvdError::InvalidVersion, "Invalid version in object '{}', only 1 is supported", oname); - PR_RET_IF(hdr->type != OBJ_SUPERBLOCK, std::nullopt, + PR_RET_IF(hdr->type != OBJ_SUPERBLOCK, LsvdError::InvalidObjectType, "Obj '{}' not a superblock", oname); parsed_superblock ret; @@ -189,22 +189,21 @@ opt object_reader::read_superblock(std::string oname) ret.snaps = deserialise_ptrs_with_len(hbuf, shdr->snaps_offset, shdr->snaps_len); - ret.superblock_buf = *buf; + ret.superblock_buf = buf; uuid_copy(ret.uuid, hdr->vol_uuid); ret.vol_size = shdr->vol_size * 512; return ret; } -opt object_reader::read_data_hdr(std::string oname) +result object_reader::read_data_hdr(std::string oname) { - auto hdr = fetch_object_header(oname); - PASSTHRU_NULLOPT(hdr); + auto hdr = BOOST_OUTCOME_TRYX(fetch_object_header(oname)); parsed_data_hdr h; - h.buf = std::move(*hdr); + h.buf = std::move(hdr); h.hdr = (common_obj_hdr *)h.buf.data(); - PR_RET_IF(h.hdr->type != OBJ_LOGDATA, std::nullopt, + PR_RET_IF(h.hdr->type != OBJ_LOGDATA, LsvdError::InvalidObjectType, "Invalid object type in '{}'", oname); h.data_hdr = (obj_data_hdr *)(h.buf.data() + sizeof(common_obj_hdr)); @@ -216,18 +215,19 @@ opt object_reader::read_data_hdr(std::string oname) return h; } -opt object_reader::read_checkpoint(std::string oname) +result object_reader::read_checkpoint(std::string oname) { - auto hdr = fetch_object_header(oname); - PASSTHRU_NULLOPT(hdr); + auto hdr = BOOST_OUTCOME_TRYX(fetch_object_header(oname)); parsed_checkpoint ret; - ret.hdr = (common_obj_hdr *)hdr->data(); - PR_RET_IF(ret.hdr->type != OBJ_CHECKPOINT, std::nullopt, + ret.buf = std::move(hdr); + + ret.hdr = (common_obj_hdr *)ret.buf.data(); + PR_RET_IF(ret.hdr->type != OBJ_CHECKPOINT, LsvdError::InvalidObjectType, "Invalid object type it '{}'", oname); - ret.ckpt_hdr = (obj_ckpt_hdr *)(&hdr->at(sizeof(common_obj_hdr))); + ret.ckpt_hdr = (obj_ckpt_hdr *)(&hdr.at(sizeof(common_obj_hdr))); - auto buf = hdr->data(); + auto buf = ret.buf.data(); ret.ckpts = deserialise_cpy(buf, ret.ckpt_hdr->ckpts_offset, ret.ckpt_hdr->ckpts_len); ret.objects = deserialise_ptrs(buf, ret.ckpt_hdr->objs_offset, @@ -236,6 +236,5 @@ opt object_reader::read_checkpoint(std::string oname) buf, ret.ckpt_hdr->deletes_offset, ret.ckpt_hdr->deletes_len); ret.dmap = deserialise_ptrs(buf, ret.ckpt_hdr->map_offset, ret.ckpt_hdr->map_len); - ret.buf = std::move(*hdr); return ret; } \ No newline at end of file diff --git a/src/objects.h b/src/objects.h index a49204de..e001c786 100644 --- a/src/objects.h +++ b/src/objects.h @@ -170,10 +170,10 @@ class object_reader public: object_reader(std::shared_ptr be) : objstore(be) {} - opt> fetch_object_header(std::string oname); - opt read_superblock(std::string oname); - opt read_data_hdr(std::string oname); - opt read_checkpoint(std::string oname); + result> fetch_object_header(str oname); + result read_superblock(str oname); + result read_data_hdr(str oname); + result read_checkpoint(str oname); }; // ----- common image types, temporary(T&Cs apply) workaround ----- diff --git a/src/rados_backend.cc b/src/rados_backend.cc index 74ab05a0..f55b03dc 100644 --- a/src/rados_backend.cc +++ b/src/rados_backend.cc @@ -1,3 +1,4 @@ +#include #include #include @@ -46,13 +47,16 @@ class rados_io_req : public self_refcount_request void notify(request *parent) override {} - int run_getres_and_free() + result run_getres_and_free() { run(nullptr); wait(); auto r = this->ret; dec_and_free(); - return r; + if (r < 0) + return errcode_to_result(-r); + else + return r; } }; @@ -72,8 +76,8 @@ class rados_read_req : public rados_io_req void run(request *parent) override { this->parent = parent; - int rv = ctx.aio_read(name, comp, &bl, iov.bytes(), off); - check_ret_neg(rv, "Failed to start RADOS aio read"); + auto rv = ctx.aio_read(name, comp, &bl, iov.bytes(), off); + THROW_ERRNO_ON(rv < 0, -rv, "Failed to start RADOS aio read"); } void notify(request *parent) override @@ -92,15 +96,15 @@ class rados_write_req : public rados_io_req : rados_io_req(name, ctx) { auto [v, c] = iov.c_iov(); - for (int i = 0; i < c; i++) + for (auto i = 0; i < c; i++) bl.append((char *)v[i].iov_base, v[i].iov_len); } void run(request *parent) override { this->parent = parent; - int rv = ctx.aio_write(name, comp, bl, bl.length(), 0); - check_ret_neg(rv, "Failed to start RADOS aio write"); + auto rv = ctx.aio_write(name, comp, bl, bl.length(), 0); + THROW_ERRNO_ON(rv < 0, -rv, "Failed to start RADOS aio write"); } }; @@ -115,8 +119,8 @@ class rados_delete_req : public rados_io_req void run(request *parent) override { this->parent = parent; - int rv = ctx.aio_remove(name, comp); - check_ret_neg(rv, "Failed to start RADOS aio remove"); + auto rv = ctx.aio_remove(name, comp); + THROW_ERRNO_ON(rv < 0, -rv, "Failed to start RADOS aio remove"); } }; @@ -133,22 +137,24 @@ class rados_backend : public backend ~rados_backend() override {} - int write(std::string name, smartiov &iov) override + result write(std::string name, smartiov &iov) override { auto req = dynamic_cast(aio_write(name, iov)); return req->run_getres_and_free(); } - int read(std::string name, off_t offset, smartiov &iov) override + result read(std::string name, off_t offset, smartiov &iov) override { auto req = dynamic_cast(aio_read(name, offset, iov)); return req->run_getres_and_free(); } - int delete_obj(std::string name) override + result delete_obj(std::string name) override { auto req = dynamic_cast(aio_delete(name)); - return req->run_getres_and_free(); + auto rc = BOOST_OUTCOME_TRYX(req->run_getres_and_free()); + assert(rc == 0); + return outcome::success(); } request *aio_write(std::string name, smartiov &iov) override @@ -171,32 +177,24 @@ class rados_backend : public backend return ctx.stat(name, nullptr, nullptr) == 0; } - opt get_size(std::string name) override + result get_size(std::string name) override { u64 size; time_t mtime; int rv = ctx.stat(name, &size, &mtime); - switch (rv) { - case 0: - return size; - case -ENOENT: - return std::nullopt; - default: - THROW_ERRNO_ON(true, -rv, "Failed to stat object '{}'", name); - } + FAILURE_IF_NEGATIVE(rv); + return size; } - opt> read_whole_obj(std::string name) override + result> read_whole_obj(std::string name) override { - auto size = get_size(name); - PASSTHRU_NULLOPT(size); + auto size = BOOST_OUTCOME_TRYX(get_size(name)); - vec buf(size.value()); + vec buf(size); smartiov iov((char *)buf.data(), buf.size()); - auto r = read(name, 0, iov); - if (r < 0) - return std::nullopt; - + auto r = BOOST_OUTCOME_TRYX(read(name, 0, iov)); + if (std::cmp_less(r, size)) + return outcome::failure(LsvdError::MissingData); return buf; } }; @@ -206,21 +204,21 @@ std::shared_ptr make_rados_backend(rados_ioctx_t io) return std::make_shared(io); } -rados_ioctx_t connect_to_pool(str pool_name) +result connect_to_pool(str pool_name) { rados_t cluster; - int err = rados_create2(&cluster, "ceph", "client.admin", 0); - check_ret_neg(err, "Failed to create cluster handle"); + auto err = rados_create2(&cluster, "ceph", "client.admin", 0); + PR_FAILURE_IF_NEGATIVE(err, "Failed to create cluster handle"); err = rados_conf_read_file(cluster, "/etc/ceph/ceph.conf"); - check_ret_neg(err, "Failed to read config file"); + PR_FAILURE_IF_NEGATIVE(err, "Failed to read config file"); err = rados_connect(cluster); - check_ret_neg(err, "Failed to connect to cluster"); + PR_FAILURE_IF_NEGATIVE(err, "Failed to connect to cluster"); rados_ioctx_t io_ctx; err = rados_ioctx_create(cluster, pool_name.c_str(), &io_ctx); - check_ret_neg(err, "Failed to connect to pool {}", pool_name); + PR_FAILURE_IF_NEGATIVE(err, "Failed to connect to pool {}", pool_name); return io_ctx; } diff --git a/src/spdk_frontend.cc b/src/spdk_frontend.cc index 5c07e8c1..1bc5c11d 100644 --- a/src/spdk_frontend.cc +++ b/src/spdk_frontend.cc @@ -172,7 +172,7 @@ static void start_lsvd(void *arg) log_info("Starting LSVD SPDK program ..."); auto args = (start_lsvd_args *)arg; - auto io_ctx = connect_to_pool(args->pool_name); + auto io_ctx = connect_to_pool(args->pool_name).value(); // Setup spdk nvmf auto tgt = create_target(); @@ -184,8 +184,7 @@ static void start_lsvd(void *arg) auto cfg = lsvd_config::get_default(); // TODO read this in from a config file cfg.rcache_bytes = 160 * 1024 * 1024; // small 160mb cache for testing - auto err = bdev_lsvd_create(args->image_name, io_ctx, cfg); - assert(err == 0); + bdev_lsvd_create(args->image_name, io_ctx, cfg); add_bdev_ns(nvme_ss, args->image_name); // some stupid formatting decisions up ahead due to tower-of-callback diff --git a/src/spdk_wrap.h b/src/spdk_wrap.h index 82b7ac09..38b9d2c2 100644 --- a/src/spdk_wrap.h +++ b/src/spdk_wrap.h @@ -1,6 +1,7 @@ #pragma once #include +#include #include "fake_rbd.h" #include "image.h" diff --git a/src/utils.h b/src/utils.h index 63d9daa9..3c31b793 100644 --- a/src/utils.h +++ b/src/utils.h @@ -1,20 +1,17 @@ #pragma once // #include "folly/FBVector.h" +#include #include #include -#include #include #include #include #include #include #include -#include #include -#include #include -#include #include #include #include @@ -28,6 +25,9 @@ #define LOGLV 1 #endif +namespace outcome = boost::outcome_v2; +template using result = outcome::result; + template using sptr = std::shared_ptr; template using uptr = std::unique_ptr; template using opt = std::optional; @@ -64,6 +64,13 @@ using fspath = std::filesystem::path; } \ } while (0) +#define PASSTHRU_FAILURE(res) \ + do { \ + if (res.has_error()) { \ + return res; \ + } \ + } while (0) + #define trace(MSG, ...) \ do { \ if (LOGLV <= 0) \ @@ -124,14 +131,6 @@ using fspath = std::filesystem::path; } \ } while (0) -#define PR_GOTO_IF(cond, lbl, MSG, ...) \ - do { \ - if (cond) { \ - log_error(MSG, ##__VA_ARGS__); \ - goto lbl; \ - } \ - } while (0) - /** * If `cond` is true, print an error message to stdout with MSG, then return * `ret` @@ -183,19 +182,28 @@ using fspath = std::filesystem::path; } \ } while (0) +#define FAILURE_IF_NEGATIVE(rc) \ + do { \ + if (rc < 0) { \ + return outcome::failure( \ + std::error_code(-rc, std::generic_category())); \ + } \ + } while (0) + /** * Check return values of functions that return -errno in the case of an error - * and throw an exception + * and convert it to a outcome::failure() instead on failure */ -#define check_ret_neg(ret, MSG, ...) \ +#define PR_FAILURE_IF_NEGATIVE(ret, MSG, ...) \ do { \ if (ret < 0) { \ auto en = -ret; \ + auto ec = std::error_code(en, std::generic_category()); \ auto fs = fmt::format(MSG "\n", ##__VA_ARGS__); \ auto s = fmt::format("[ERR {}:{} {} | errno {}/{}] {}", __FILE__, \ - __LINE__, __func__, en, strerror(en), fs); \ + __LINE__, __func__, en, ec.message(), fs); \ fmt::print(stderr, fg(fmt::color::red) | fmt::emphasis::bold, s); \ - throw std::runtime_error(fs); \ + return outcome::failure(ec); \ } \ } while (0) @@ -286,3 +294,83 @@ inline size_t getsize64(int fd) size = sb.st_size; return size; } + +template inline auto errcode_to_result(int rc) -> result +{ + if (rc > 0) + return outcome::failure(std::error_code(rc, std::generic_category())); + return outcome::success(); +} + +enum class LsvdError { + Success, + InvalidMagic, + InvalidVersion, + InvalidObjectType, + MissingData, +}; + +namespace std +{ +template <> struct is_error_code_enum : true_type { +}; +}; // namespace std + +namespace detail +{ +// Define a custom error code category derived from std::error_category +class BackendError_category : public std::error_category +{ + public: + // Return a short descriptive name for the category + virtual const char *name() const noexcept override final + { + return "Data validity error"; + } + + // Return what each enum means in text + virtual std::string message(int c) const override final + { + switch (static_cast(c)) { + case LsvdError::Success: + return "passed validity checks"; + case LsvdError::InvalidMagic: + return "invalid magic number in the header"; + case LsvdError::InvalidVersion: + return "version number unsupported"; + case LsvdError::InvalidObjectType: + return "does not match required object type"; + case LsvdError::MissingData: + return "required data not found"; + } + } +}; +} // namespace detail + +extern inline const detail::BackendError_category &BackendError_category() +{ + static detail::BackendError_category c; + return c; +} + +inline std::error_code make_error_code(LsvdError e) +{ + static detail::BackendError_category c; + return {static_cast(e), c}; +} + +inline int result_to_rc(result res) +{ + if (res.has_value()) + return 0; + else + return res.error().value(); +} + +inline int result_to_rc(result res) +{ + if (res.has_value()) + return res.value(); + else + return res.error().value(); +} diff --git a/test/test-seq.cc b/test/test-seq.cc index 66aae582..ba1d6add 100644 --- a/test/test-seq.cc +++ b/test/test-seq.cc @@ -3,6 +3,7 @@ #include #include +#include "backend.h" #include "fake_rbd.h" #include "utils.h" @@ -176,29 +177,16 @@ void run_test(rados_ioctx_t ctx) int main(int argc, char *argv[]) { // config options + // TODO set this with new config framework setenv("LSVD_RCACHE_DIR", "/tmp/lsvd", 1); setenv("LSVD_WCACHE_DIR", "/tmp/lsvd", 1); setenv("LSVD_CACHE_SIZE", "2147483648", 1); std::string pool_name = "pone"; - - rados_t cluster; - int err = rados_create2(&cluster, "ceph", "client.admin", 0); - check_ret_neg(err, "Failed to create cluster handle"); - - err = rados_conf_read_file(cluster, "/etc/ceph/ceph.conf"); - check_ret_neg(err, "Failed to read config file"); - - err = rados_connect(cluster); - check_ret_neg(err, "Failed to connect to cluster"); - - rados_ioctx_t io_ctx; - err = rados_ioctx_create(cluster, pool_name.c_str(), &io_ctx); - check_ret_neg(err, "Failed to connect to pool {}", pool_name); + auto io_ctx = connect_to_pool(pool_name).value(); run_test(io_ctx); rados_ioctx_destroy(io_ctx); - rados_shutdown(cluster); return 0; }