From: Jason Dillaman Date: Thu, 6 Oct 2016 14:57:21 +0000 (-0400) Subject: librbd: relocate image option processing to create state machine X-Git-Tag: v11.1.0~701^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=b083fa0c933c1d69e315daf56ecffa64295cc8f2;p=ceph.git librbd: relocate image option processing to create state machine Replace the individual image option parameters with the ImageOptions wrapper and extract the options within the state machine itself. This will simplify adding support for the data pool and other options in the future. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/image/CreateRequest.cc b/src/librbd/image/CreateRequest.cc index 4a1378cdc08..df0fe28f079 100644 --- a/src/librbd/image/CreateRequest.cc +++ b/src/librbd/image/CreateRequest.cc @@ -27,29 +27,67 @@ using util::create_context_callback; namespace { -bool validate_features(CephContext *cct, uint64_t features, bool force_non_primary) { +int validate_features(CephContext *cct, uint64_t features, + bool force_non_primary) { + if (features & ~RBD_FEATURES_ALL) { + lderr(cct) << "librbd does not support requested features." << dendl; + return -ENOSYS; + } if ((features & RBD_FEATURE_FAST_DIFF) != 0 && (features & RBD_FEATURE_OBJECT_MAP) == 0) { lderr(cct) << "cannot use fast diff without object map" << dendl; - return false; + return -EINVAL; } if ((features & RBD_FEATURE_OBJECT_MAP) != 0 && (features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0) { lderr(cct) << "cannot use object map without exclusive lock" << dendl; - return false; + return -EINVAL; } if ((features & RBD_FEATURE_JOURNALING) != 0) { if ((features & RBD_FEATURE_EXCLUSIVE_LOCK) == 0) { lderr(cct) << "cannot use journaling without exclusive lock" << dendl; - return false; + return -EINVAL; } } else if (force_non_primary) { assert(false); } - return true; + return 0; +} + +int validate_striping(CephContext *cct, uint8_t order, uint64_t stripe_unit, + uint64_t stripe_count) { + if ((stripe_unit && !stripe_count) || + (!stripe_unit && stripe_count)) { + lderr(cct) << "must specify both (or neither) of stripe-unit and " + << "stripe-count" << dendl; + return -EINVAL; + } else if (stripe_unit || stripe_count) { + if ((1ull << order) % stripe_unit || stripe_unit > (1ull << order)) { + lderr(cct) << "stripe unit is not a factor of the object size" << dendl; + return -EINVAL; + } + } + return 0; } +int validate_data_pool(CephContext *cct, IoCtx &io_ctx, uint64_t features, + const std::string &data_pool) { + if ((features & RBD_FEATURE_DATA_POOL) == 0) { + return 0; + } + + librados::Rados rados(io_ctx); + librados::IoCtx data_io_ctx; + int r = rados.ioctx_create(data_pool.c_str(), data_io_ctx); + if (r < 0) { + lderr(cct) << "data pool " << data_pool << " does not exist" << dendl; + return -ENOENT; + } + return 0; +} + + bool validate_layout(CephContext *cct, uint64_t size, file_layout_t &layout) { if (!librbd::ObjectMap::is_compatible(layout, size)) { lderr(cct) << "image size not compatible with object map" << dendl; @@ -59,6 +97,17 @@ bool validate_layout(CephContext *cct, uint64_t size, file_layout_t &layout) { return true; } +int get_image_option(const ImageOptions &image_options, int option, + uint8_t *value) { + uint64_t large_value; + int r = image_options.get(option, &large_value); + if (r < 0) { + return r; + } + *value = static_cast(large_value); + return 0; +} + } // anonymous namespace // TODO: do away with @m_op_work_queue @@ -66,21 +115,16 @@ bool validate_layout(CephContext *cct, uint64_t size, file_layout_t &layout) { // worker thread (see callers of ->queue()). Once everything is made // fully asynchronous this can be done away with. template -CreateRequest::CreateRequest(IoCtx &ioctx, std::string &imgname, std::string &imageid, - uint64_t size, int order, uint64_t features, - uint64_t stripe_unit, uint64_t stripe_count, - uint8_t journal_order, uint8_t journal_splay_width, - const std::string &journal_pool, +CreateRequest::CreateRequest(IoCtx &ioctx, const std::string &image_name, + const std::string &image_id, uint64_t size, + const ImageOptions &image_options, const std::string &non_primary_global_image_id, const std::string &primary_mirror_uuid, - ContextWQ *op_work_queue, Context *on_finish) : - m_image_name(imgname), m_image_id(imageid), m_size(size), m_order(order), - m_features(features), m_stripe_unit(stripe_unit), m_stripe_count(stripe_count), - m_journal_order(journal_order), m_journal_splay_width(journal_splay_width), - m_journal_pool(journal_pool), m_non_primary_global_image_id(non_primary_global_image_id), - m_primary_mirror_uuid(primary_mirror_uuid), - m_op_work_queue(op_work_queue), m_on_finish(on_finish) { - + ContextWQ *op_work_queue, Context *on_finish) + : m_image_name(image_name), m_image_id(image_id), m_size(size), + m_non_primary_global_image_id(non_primary_global_image_id), + m_primary_mirror_uuid(primary_mirror_uuid), + m_op_work_queue(op_work_queue), m_on_finish(on_finish) { m_ioctx.dup(ioctx); m_cct = reinterpret_cast(m_ioctx.cct()); @@ -88,6 +132,48 @@ CreateRequest::CreateRequest(IoCtx &ioctx, std::string &imgname, std::string m_header_obj = util::header_name(m_image_id); m_objmap_name = ObjectMap::object_map_name(m_image_id, CEPH_NOSNAP); + if (image_options.get(RBD_IMAGE_OPTION_FEATURES, &m_features) != 0) { + m_features = m_cct->_conf->rbd_default_features; + } + + uint64_t features_clear = 0; + uint64_t features_set = 0; + image_options.get(RBD_IMAGE_OPTION_FEATURES_CLEAR, &features_clear); + image_options.get(RBD_IMAGE_OPTION_FEATURES_SET, &features_set); + + uint64_t features_conflict = features_clear & features_set; + features_clear &= ~features_conflict; + features_set &= ~features_conflict; + m_features |= features_set; + m_features &= ~features_clear; + + if (image_options.get(RBD_IMAGE_OPTION_STRIPE_UNIT, &m_stripe_unit) != 0 || + m_stripe_unit == 0) { + m_stripe_unit = m_cct->_conf->rbd_default_stripe_unit; + } + if (image_options.get(RBD_IMAGE_OPTION_STRIPE_COUNT, &m_stripe_count) != 0 || + m_stripe_count == 0) { + m_stripe_count = m_cct->_conf->rbd_default_stripe_count; + } + if (get_image_option(image_options, RBD_IMAGE_OPTION_ORDER, &m_order) != 0 || + m_order == 0) { + m_order = m_cct->_conf->rbd_default_order; + } + if (get_image_option(image_options, RBD_IMAGE_OPTION_JOURNAL_ORDER, + &m_journal_order) != 0) { + m_journal_order = m_cct->_conf->rbd_journal_order; + } + if (get_image_option(image_options, RBD_IMAGE_OPTION_JOURNAL_SPLAY_WIDTH, + &m_journal_splay_width) != 0) { + m_journal_splay_width = m_cct->_conf->rbd_journal_splay_width; + } + if (image_options.get(RBD_IMAGE_OPTION_JOURNAL_POOL, &m_journal_pool) != 0) { + m_journal_pool = m_cct->_conf->rbd_journal_pool; + } + if (image_options.get(RBD_IMAGE_OPTION_DATA_POOL, &m_data_pool) != 0) { + m_data_pool = m_cct->_conf->rbd_default_data_pool; + } + m_layout.object_size = 1ull << m_order; if (m_stripe_unit == 0 || m_stripe_count == 0) { m_layout.stripe_unit = m_layout.object_size; @@ -99,18 +185,69 @@ CreateRequest::CreateRequest(IoCtx &ioctx, std::string &imgname, std::string m_force_non_primary = !non_primary_global_image_id.empty(); - // TODO - m_features &= ~RBD_FEATURE_DATA_POOL; + if (/* TODO */ false && !m_data_pool.empty() && m_data_pool != ioctx.get_pool_name()) { + m_features |= RBD_FEATURE_DATA_POOL; + } else { + m_features &= ~RBD_FEATURE_DATA_POOL; + } + + if ((m_stripe_unit != 0 && m_stripe_unit != (1ULL << m_order)) || + (m_stripe_count != 0 && m_stripe_count != 1)) { + m_features |= RBD_FEATURE_STRIPINGV2; + } else { + m_features &= ~RBD_FEATURE_STRIPINGV2; + } + + ldout(m_cct, 20) << "name=" << m_image_name << ", " + << "id=" << m_image_id << ", " + << "size=" << m_size << ", " + << "features=" << m_features << ", " + << "order=" << m_order << ", " + << "stripe_unit=" << m_stripe_unit << ", " + << "stripe_count=" << m_stripe_count << ", " + << "journal_order=" << m_journal_order << ", " + << "journal_splay_width=" << m_journal_splay_width << ", " + << "journal_pool=" << m_journal_pool << ", " + << "data_pool=" << m_data_pool << dendl; +} + +template +int CreateRequest::validate_order(CephContext *cct, uint8_t order) { + if (order > 25 || order < 12) { + lderr(cct) << "order must be in the range [12, 25]" << dendl; + return -EDOM; + } + return 0; } template void CreateRequest::send() { ldout(m_cct, 20) << this << " " << __func__ << dendl; - if (!validate_features(m_cct, m_features, m_force_non_primary)) { - complete(-EINVAL); + int r = validate_features(m_cct, m_features, m_force_non_primary); + if (r < 0) { + complete(r); + return; + } + + r = validate_order(m_cct, m_order); + if (r < 0) { + complete(r); + return; + } + + r = validate_striping(m_cct, m_order, m_stripe_unit, m_stripe_count); + if (r < 0) { + complete(r); return; } + + r = validate_data_pool(m_cct, m_ioctx, m_features, m_data_pool); + if (r < 0) { + complete(r); + return; + } + if (!validate_layout(m_cct, m_size, m_layout)) { complete(-EINVAL); return; diff --git a/src/librbd/image/CreateRequest.h b/src/librbd/image/CreateRequest.h index 16288823fa4..90941129104 100644 --- a/src/librbd/image/CreateRequest.h +++ b/src/librbd/image/CreateRequest.h @@ -30,20 +30,19 @@ namespace image { template class CreateRequest { public: - static CreateRequest *create(IoCtx &ioctx, std::string &imgname, std::string &imageid, - uint64_t size, int order, uint64_t features, - uint64_t stripe_unit, uint64_t stripe_count, - uint8_t journal_order, uint8_t journal_splay_width, - const std::string &journal_pool, + static CreateRequest *create(IoCtx &ioctx, const std::string &image_name, + const std::string &image_id, uint64_t size, + const ImageOptions &image_options, const std::string &non_primary_global_image_id, const std::string &primary_mirror_uuid, ContextWQ *op_work_queue, Context *on_finish) { - return new CreateRequest(ioctx, imgname, imageid, size, order, features, stripe_unit, - stripe_count, journal_order, journal_splay_width, journal_pool, + return new CreateRequest(ioctx, image_name, image_id, size, image_options, non_primary_global_image_id, primary_mirror_uuid, op_work_queue, on_finish); } + static int validate_order(CephContext *cct, uint8_t order); + void send(); private: @@ -91,10 +90,9 @@ private: * @endverbatim */ - CreateRequest(IoCtx &ioctx, std::string &imgname, std::string &imageid, - uint64_t size, int order, uint64_t features, - uint64_t stripe_unit, uint64_t stripe_count, uint8_t journal_order, - uint8_t journal_splay_width, const std::string &journal_pool, + CreateRequest(IoCtx &ioctx, const std::string &image_name, + const std::string &image_id, uint64_t size, + const ImageOptions &image_options, const std::string &non_primary_global_image_id, const std::string &primary_mirror_uuid, ContextWQ *op_work_queue, Context *on_finish); @@ -103,10 +101,14 @@ private: std::string m_image_name; std::string m_image_id; uint64_t m_size; - int m_order; - uint64_t m_features, m_stripe_unit, m_stripe_count; - uint8_t m_journal_order, m_journal_splay_width; - const std::string m_journal_pool; + uint8_t m_order = 0; + uint64_t m_features = 0; + uint64_t m_stripe_unit = 0; + uint64_t m_stripe_count = 0; + uint8_t m_journal_order = 0; + uint8_t m_journal_splay_width = 0; + std::string m_journal_pool; + std::string m_data_pool; const std::string m_non_primary_global_image_id; const std::string m_primary_mirror_uuid; diff --git a/src/librbd/internal.cc b/src/librbd/internal.cc index f01c15f5514..00b647f88d2 100644 --- a/src/librbd/internal.cc +++ b/src/librbd/internal.cc @@ -836,6 +836,8 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, { CephContext *cct = (CephContext *)io_ctx.cct(); + ldout(cct, 20) << __func__ << " " << &io_ctx << " name = " << imgname + << " size = " << size << " order = " << order << dendl; int r = validate_pool(io_ctx, cct); if (r < 0) { return r; @@ -877,21 +879,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, return 0; } - void create_v2(IoCtx& io_ctx, std::string &imgname, uint64_t size, - int order, uint64_t features, uint64_t stripe_unit, - uint64_t stripe_count, uint8_t journal_order, - uint8_t journal_splay_width, const std::string &journal_pool, - const std::string &non_primary_global_image_id, - const std::string &primary_mirror_uuid, - ContextWQ *op_work_queue, Context *ctx) { - std::string id = util::generate_image_id(io_ctx); - image::CreateRequest<> *req = image::CreateRequest<>::create( - io_ctx, imgname, id, size, order, features, stripe_unit, - stripe_count, journal_order, journal_splay_width, journal_pool, - non_primary_global_image_id, primary_mirror_uuid, op_work_queue, ctx); - req->send(); - } - int create(librados::IoCtx& io_ctx, const char *imgname, uint64_t size, int *order) { @@ -947,47 +934,6 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, format = cct->_conf->rbd_default_format; bool old_format = format == 1; - uint64_t features; - if (opts.get(RBD_IMAGE_OPTION_FEATURES, &features) != 0) { - features = old_format ? 0 : cct->_conf->rbd_default_features; - } - - uint64_t features_clear = 0; - uint64_t features_set = 0; - opts.get(RBD_IMAGE_OPTION_FEATURES_CLEAR, &features_clear); - opts.get(RBD_IMAGE_OPTION_FEATURES_SET, &features_set); - - uint64_t conflict = features_clear & features_set; - features_clear &= ~conflict; - features_set &= ~conflict; - - features |= features_set; - features &= ~features_clear; - - uint64_t stripe_unit = 0; - uint64_t stripe_count = 0; - if (!old_format) { - if (opts.get(RBD_IMAGE_OPTION_STRIPE_UNIT, &stripe_unit) != 0 || stripe_unit == 0) - stripe_unit = cct->_conf->rbd_default_stripe_unit; - if (opts.get(RBD_IMAGE_OPTION_STRIPE_COUNT, &stripe_count) != 0 || stripe_count == 0) - stripe_count = cct->_conf->rbd_default_stripe_count; - } - - uint64_t order = 0; - if (opts.get(RBD_IMAGE_OPTION_ORDER, &order) != 0 || order == 0) - order = cct->_conf->rbd_default_order; - - ldout(cct, 20) << "create " << &io_ctx << " name = " << imgname - << " size = " << size << " old_format = " << old_format - << " features = " << features << " order = " << order - << " stripe_unit = " << stripe_unit - << " stripe_count = " << stripe_count - << dendl; - - if (features & ~RBD_FEATURES_ALL) { - lderr(cct) << "librbd does not support requested features." << dendl; - return -ENOSYS; - } // make sure it doesn't already exist, in either format int r = detect_format(io_ctx, imgname, NULL, NULL); @@ -1000,53 +946,29 @@ int mirror_image_disable_internal(ImageCtx *ictx, bool force, return -EEXIST; } - if (order > 25 || order < 12) { - lderr(cct) << "order must be in the range [12, 25]" << dendl; - return -EDOM; - } - - if ((features & RBD_FEATURE_STRIPINGV2) == 0 && - ((stripe_unit && stripe_unit != (1ull << order)) || - (stripe_count && stripe_count != 1))) { - features |= RBD_FEATURE_STRIPINGV2; + uint64_t order = 0; + if (opts.get(RBD_IMAGE_OPTION_ORDER, &order) != 0 || order == 0) { + order = cct->_conf->rbd_default_order; } - - if ((stripe_unit && !stripe_count) || - (!stripe_unit && stripe_count)) { - lderr(cct) << "must specify both (or neither) of stripe-unit and stripe-count" << dendl; - return -EINVAL; - } else if (stripe_unit || stripe_count) { - if ((1ull << order) % stripe_unit || stripe_unit > (1ull << order)) { - lderr(cct) << "stripe unit is not a factor of the object size" << dendl; - return -EINVAL; - } + r = image::CreateRequest<>::validate_order(cct, order); + if (r < 0) { + return r; } if (old_format) { - if (stripe_unit && stripe_unit != (1ull << order)) - return -EINVAL; - if (stripe_count && stripe_count != 1) - return -EINVAL; - r = create_v1(io_ctx, imgname, size, order); } else { - uint64_t journal_order = cct->_conf->rbd_journal_order; - uint64_t journal_splay_width = cct->_conf->rbd_journal_splay_width; - std::string journal_pool = cct->_conf->rbd_journal_pool; - - opts.get(RBD_IMAGE_OPTION_JOURNAL_ORDER, &journal_order); - opts.get(RBD_IMAGE_OPTION_JOURNAL_SPLAY_WIDTH, &journal_splay_width); - opts.get(RBD_IMAGE_OPTION_JOURNAL_POOL, &journal_pool); - C_SaferCond cond; ContextWQ op_work_queue("librbd::op_work_queue", cct->_conf->rbd_op_thread_timeout, ImageCtx::get_thread_pool_instance(cct)); - std::string imagename(imgname); - create_v2(io_ctx, imagename, size, order, features, stripe_unit, - stripe_count, journal_order, journal_splay_width, journal_pool, - non_primary_global_image_id, primary_mirror_uuid, - &op_work_queue, &cond); + + std::string id = util::generate_image_id(io_ctx); + image::CreateRequest<> *req = image::CreateRequest<>::create( + io_ctx, imgname, id, size, opts, non_primary_global_image_id, + primary_mirror_uuid, &op_work_queue, &cond); + req->send(); + r = cond.wait(); op_work_queue.drain(); } diff --git a/src/librbd/internal.h b/src/librbd/internal.h index 01b95464f6a..ddc1cb06560 100644 --- a/src/librbd/internal.h +++ b/src/librbd/internal.h @@ -98,13 +98,6 @@ namespace librbd { std::set > & names); int list_children_info(ImageCtx *ictx, librbd::parent_spec parent_spec, std::map, std::set >& image_info); - void create_v2(IoCtx& io_ctx, std::string &imgname, uint64_t size, - int order, uint64_t features, uint64_t stripe_unit, - uint64_t stripe_count, uint8_t journal_order, - uint8_t journal_splay_width, const std::string &journal_pool, - const std::string &non_primary_global_image_id, - const std::string &primary_mirror_uuid, - ContextWQ *op_work_queue, Context *ctx); int create(librados::IoCtx& io_ctx, const char *imgname, uint64_t size, int *order); int create(librados::IoCtx& io_ctx, const char *imgname, uint64_t size, diff --git a/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc b/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc index 8399b4b3426..e4bee3cec78 100644 --- a/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc +++ b/src/test/rbd_mirror/image_replayer/test_mock_CreateImageRequest.cc @@ -35,15 +35,13 @@ struct CreateRequest { static CreateRequest *s_instance; Context *on_finish = nullptr; - static CreateRequest *create(IoCtx &ioctx, std::string &imgname, - std::string &imageid, uint64_t size, int order, - uint64_t features, uint64_t stripe_unit, - uint64_t stripe_count, uint8_t journal_order, - uint8_t journal_splay_width, - const std::string &journal_pool, + static CreateRequest *create(IoCtx &ioctx, const std::string &imgname, + const std::string &imageid, uint64_t size, + const librbd::ImageOptions &image_options, const std::string &non_primary_global_image_id, const std::string &primary_mirror_uuid, - MockContextWQ *op_work_queue, Context *on_finish) { + MockContextWQ *op_work_queue, + Context *on_finish) { assert(s_instance != nullptr); s_instance->on_finish = on_finish; s_instance->construct(ioctx); diff --git a/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc b/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc index 448069a9ff1..27c6c1ca2bd 100644 --- a/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc +++ b/src/tools/rbd_mirror/image_replayer/CreateImageRequest.cc @@ -64,19 +64,20 @@ void CreateImageRequest::create_image() { Context *ctx = create_context_callback(this); RWLock::RLocker snap_locker(m_remote_image_ctx->snap_lock); - int order = m_remote_image_ctx->order; - CephContext *cct = reinterpret_cast(m_local_io_ctx.cct()); - uint64_t journal_order = cct->_conf->rbd_journal_order; - uint64_t journal_splay_width = cct->_conf->rbd_journal_splay_width; - std::string journal_pool = cct->_conf->rbd_journal_pool; + librbd::ImageOptions image_options; + image_options.set(RBD_IMAGE_OPTION_FEATURES, m_remote_image_ctx->features); + image_options.set(RBD_IMAGE_OPTION_ORDER, m_remote_image_ctx->order); + image_options.set(RBD_IMAGE_OPTION_STRIPE_UNIT, + m_remote_image_ctx->stripe_unit); + image_options.set(RBD_IMAGE_OPTION_STRIPE_COUNT, + m_remote_image_ctx->stripe_count); + std::string id = librbd::util::generate_image_id(m_local_io_ctx); librbd::image::CreateRequest *req = librbd::image::CreateRequest::create( m_local_io_ctx, m_local_image_name, id, m_remote_image_ctx->size, - order, m_remote_image_ctx->features, m_remote_image_ctx->stripe_unit, - m_remote_image_ctx->stripe_count, journal_order, journal_splay_width, - journal_pool, m_global_image_id, m_remote_mirror_uuid, + image_options, m_global_image_id, m_remote_mirror_uuid, m_remote_image_ctx->op_work_queue, ctx); req->send(); }