librbd/migration: require snapshot when importing from native source

Since we cannot mark the source image read-only when running in import-only
migration mode, we should require the user to provide a snapshot to ensure
that data cannot change while the migration is running.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
This commit is contained in:
Jason Dillaman 2020-12-22 09:46:04 -05:00
parent 77b8a7f8cc
commit d22ca3d978
6 changed files with 170 additions and 44 deletions

View File

@ -75,13 +75,17 @@ test_import_native_format() {
local base_image=$1
local dest_image=$2
rbd migration prepare --import-only "rbd/${base_image}@2" ${dest_image}
rbd migration abort ${dest_image}
local pool_id=$(ceph osd pool ls detail --format xml | xmlstarlet sel -t -v "//pools/pool[pool_name='rbd']/pool_id")
cat > ${TEMPDIR}/spec.json <<EOF
{
"type": "native",
"pool_id": ${pool_id},
"pool_namespace": "",
"image_name": "${base_image}"
"image_name": "${base_image}",
"snap_name": "2"
}
EOF
cat ${TEMPDIR}/spec.json
@ -91,11 +95,6 @@ EOF
compare_images "${base_image}@1" "${dest_image}@1"
compare_images "${base_image}@2" "${dest_image}@2"
compare_images "${base_image}" "${dest_image}"
rbd snap create ${dest_image}@head
rbd bench --io-type write --io-pattern rand --io-size=32K --io-total=32M ${dest_image}
compare_images "${base_image}" "${dest_image}@head"
rbd migration abort ${dest_image}
@ -105,24 +104,22 @@ EOF
compare_images "${base_image}@1" "${dest_image}@1"
compare_images "${base_image}@2" "${dest_image}@2"
compare_images "${base_image}" "${dest_image}"
rbd migration abort ${dest_image}
rbd migration prepare --import-only \
--source-spec "{\"type\": \"native\", \"pool_id\": "${pool_id}", \"image_name\": \"${base_image}\"}" \
--source-spec "{\"type\": \"native\", \"pool_id\": "${pool_id}", \"image_name\": \"${base_image}\", \"snap_name\": \"2\"}" \
${dest_image}
rbd migration abort ${dest_image}
rbd migration prepare --import-only \
--source-spec "{\"type\": \"native\", \"pool_name\": \"rbd\", \"image_name\": \"${base_image}\"}" \
--source-spec "{\"type\": \"native\", \"pool_name\": \"rbd\", \"image_name\": \"${base_image}\", \"snap_name\": \"2\"}" \
${dest_image}
rbd migration execute ${dest_image}
rbd migration commit ${dest_image}
compare_images "${base_image}@1" "${dest_image}@1"
compare_images "${base_image}@2" "${dest_image}@2"
compare_images "${base_image}" "${dest_image}"
remove_image "${dest_image}"
}

View File

@ -4,6 +4,7 @@
#include "librbd/migration/NativeFormat.h"
#include "include/neorados/RADOS.hpp"
#include "common/dout.h"
#include "common/errno.h"
#include "librbd/ImageCtx.h"
#include "librbd/ImageState.h"
#include "librbd/Utils.h"
@ -29,6 +30,8 @@ const std::string POOL_NAME_KEY{"pool_name"};
const std::string POOL_NAMESPACE_KEY{"pool_namespace"};
const std::string IMAGE_NAME_KEY{"image_name"};
const std::string IMAGE_ID_KEY{"image_id"};
const std::string SNAP_NAME_KEY{"snap_name"};
const std::string SNAP_ID_KEY{"snap_id"};
} // anonymous namespace
@ -123,6 +126,43 @@ void NativeFormat<I>::open(Context* on_finish) {
return;
}
auto& snap_name_val = m_json_object[SNAP_NAME_KEY];
if (snap_name_val.type() == json_spirit::str_type) {
m_snap_name = snap_name_val.get_str();
} else if (snap_name_val.type() != json_spirit::null_type) {
lderr(cct) << "invalid snap name" << dendl;
on_finish->complete(-EINVAL);
return;
}
auto& snap_id_val = m_json_object[SNAP_ID_KEY];
if (!m_snap_name.empty() && snap_id_val.type() != json_spirit::null_type) {
lderr(cct) << "cannot specify both snap name and snap id" << dendl;
on_finish->complete(-EINVAL);
return;
} else if (snap_id_val.type() == json_spirit::str_type) {
try {
m_snap_id = boost::lexical_cast<uint64_t>(snap_id_val.get_str());
} catch (boost::bad_lexical_cast &) {
}
} else if (snap_id_val.type() == json_spirit::int_type) {
m_snap_id = snap_id_val.get_uint64();
}
if (snap_id_val.type() != json_spirit::null_type &&
m_snap_id == CEPH_NOSNAP) {
lderr(cct) << "invalid snap id" << dendl;
on_finish->complete(-EINVAL);
return;
}
// snapshot is required for import to keep source read-only
if (m_import_only && m_snap_name.empty() && m_snap_id == CEPH_NOSNAP) {
lderr(cct) << "snapshot required for import" << dendl;
on_finish->complete(-EINVAL);
return;
}
// TODO add support for external clusters
librados::IoCtx io_ctx;
int r = util::create_ioctx(m_image_ctx->md_ctx, "source image",
@ -153,9 +193,63 @@ void NativeFormat<I>::open(Context* on_finish) {
}
// open the source RBD image
on_finish = new LambdaContext([this, on_finish](int r) {
handle_open(r, on_finish); });
m_image_ctx->state->open(flags, on_finish);
}
template <typename I>
void NativeFormat<I>::handle_open(int r, Context* on_finish) {
auto cct = m_image_ctx->cct;
ldout(cct, 10) << "r=" << r << dendl;
if (r < 0) {
lderr(cct) << "failed to open image: " << cpp_strerror(r) << dendl;
on_finish->complete(r);
return;
}
if (m_snap_id == CEPH_NOSNAP && m_snap_name.empty()) {
on_finish->complete(0);
return;
}
if (!m_snap_name.empty()) {
std::shared_lock image_locker{m_image_ctx->image_lock};
m_snap_id = m_image_ctx->get_snap_id(cls::rbd::UserSnapshotNamespace{},
m_snap_name);
}
if (m_snap_id == CEPH_NOSNAP) {
lderr(cct) << "failed to locate snapshot " << m_snap_name << dendl;
on_finish = new LambdaContext([on_finish](int) {
on_finish->complete(-ENOENT); });
m_image_ctx->state->close(on_finish);
return;
}
on_finish = new LambdaContext([this, on_finish](int r) {
handle_snap_set(r, on_finish); });
m_image_ctx->state->snap_set(m_snap_id, on_finish);
}
template <typename I>
void NativeFormat<I>::handle_snap_set(int r, Context* on_finish) {
auto cct = m_image_ctx->cct;
ldout(cct, 10) << "r=" << r << dendl;
if (r < 0) {
lderr(cct) << "failed to set snapshot " << m_snap_id << ": "
<< cpp_strerror(r) << dendl;
on_finish = new LambdaContext([r, on_finish](int) {
on_finish->complete(r); });
m_image_ctx->state->close(on_finish);
return;
}
on_finish->complete(0);
}
template <typename I>
void NativeFormat<I>::close(Context* on_finish) {
auto cct = m_image_ctx->cct;

View File

@ -66,6 +66,11 @@ private:
std::string m_pool_namespace;
std::string m_image_name;
std::string m_image_id;
std::string m_snap_name;
uint64_t m_snap_id = CEPH_NOSNAP;
void handle_open(int r, Context* on_finish);
void handle_snap_set(int r, Context* on_finish);
};

View File

@ -1491,7 +1491,7 @@
[--source-spec-path <source-spec-path>]
[--source-spec <source-spec>] [--pool <pool>]
[--namespace <namespace>] [--image <image>]
[--dest-pool <dest-pool>]
[--snap <snap>] [--dest-pool <dest-pool>]
[--dest-namespace <dest-namespace>]
[--dest <dest>] [--image-format <image-format>]
[--new-format] [--order <order>]
@ -1504,40 +1504,44 @@
[--journal-splay-width <journal-splay-width>]
[--journal-object-size <journal-object-size>]
[--journal-pool <journal-pool>] [--flatten]
<source-image-spec> <dest-image-spec>
<source-image-or-snap-spec> <dest-image-spec>
Prepare image migration.
Positional arguments
<source-image-spec> source image specification
(example: [<pool-name>/[<namespace>/]]<image-name>)
<dest-image-spec> destination image specification
(example: [<pool-name>/[<namespace>/]]<image-name>)
<source-image-or-snap-spec> source image or snapshot specification
(example:
[<pool-name>/[<namespace>/]]<image-name>[@<snap-n
ame>])
<dest-image-spec> destination image specification
(example:
[<pool-name>/[<namespace>/]]<image-name>)
Optional arguments
--import-only only import data from source
--source-spec-path arg source-spec file (or '-' for stdin)
--source-spec arg source-spec
-p [ --pool ] arg source pool name
--namespace arg source namespace name
--image arg source image name
--dest-pool arg destination pool name
--dest-namespace arg destination namespace name
--dest arg destination image name
--image-format arg image format [default: 2]
--object-size arg object size in B/K/M [4K <= object size <= 32M]
--image-feature arg image features
[layering(+), exclusive-lock(+*), object-map(+*),
deep-flatten(+-), journaling(*)]
--image-shared shared image
--stripe-unit arg stripe unit in B/K/M
--stripe-count arg stripe count
--data-pool arg data pool
--mirror-image-mode arg mirror image mode [journal or snapshot]
--journal-splay-width arg number of active journal objects
--journal-object-size arg size of journal objects [4K <= size <= 64M]
--journal-pool arg pool for journal objects
--flatten fill clone with parent data (make it independent)
--import-only only import data from source
--source-spec-path arg source-spec file (or '-' for stdin)
--source-spec arg source-spec
-p [ --pool ] arg source pool name
--namespace arg source namespace name
--image arg source image name
--snap arg source snapshot name
--dest-pool arg destination pool name
--dest-namespace arg destination namespace name
--dest arg destination image name
--image-format arg image format [default: 2]
--object-size arg object size in B/K/M [4K <= object size <= 32M]
--image-feature arg image features
[layering(+), exclusive-lock(+*),
object-map(+*), deep-flatten(+-), journaling(*)]
--image-shared shared image
--stripe-unit arg stripe unit in B/K/M
--stripe-count arg stripe count
--data-pool arg data pool
--mirror-image-mode arg mirror image mode [journal or snapshot]
--journal-splay-width arg number of active journal objects
--journal-object-size arg size of journal objects [4K <= size <= 64M]
--journal-pool arg pool for journal objects
--flatten fill clone with parent data (make it independent)
Image Features:
(*) supports enabling/disabling on existing images

View File

@ -2644,13 +2644,15 @@ class TestMigration(object):
create_image()
with Image(ioctx, image_name) as image:
image_id = image.id()
image.create_snap('snap')
source_spec = json.dumps(
{'type': 'native',
'pool_id': ioctx.get_pool_id(),
'pool_namespace': '',
'image_name': image_name,
'image_id': image_id})
'image_id': image_id,
'snap_name': 'snap'})
dst_image_name = get_temp_image_name()
RBD().migration_prepare_import(source_spec, ioctx, dst_image_name,
features=63, order=23, stripe_unit=1<<23,
@ -2667,6 +2669,12 @@ class TestMigration(object):
RBD().migration_execute(ioctx, dst_image_name)
RBD().migration_commit(ioctx, dst_image_name)
with Image(ioctx, image_name) as image:
image.remove_snap('snap')
with Image(ioctx, dst_image_name) as image:
image.remove_snap('snap')
RBD().remove(ioctx, dst_image_name)
RBD().remove(ioctx, image_name)

View File

@ -138,7 +138,8 @@ void get_prepare_arguments(po::options_description *positional,
"source-spec file (or '-' for stdin)")
("source-spec", po::value<std::string>(),
"source-spec");
at::add_image_spec_options(positional, options, at::ARGUMENT_MODIFIER_SOURCE);
at::add_image_or_snap_spec_options(positional, options,
at::ARGUMENT_MODIFIER_SOURCE);
at::add_image_spec_options(positional, options, at::ARGUMENT_MODIFIER_DEST);
at::add_create_image_options(options, true);
at::add_flatten_option(options);
@ -146,13 +147,18 @@ void get_prepare_arguments(po::options_description *positional,
int execute_prepare(const po::variables_map &vm,
const std::vector<std::string> &ceph_global_init_args) {
bool import_only = vm["import-only"].as<bool>();
size_t arg_index = 0;
std::string pool_name;
std::string namespace_name;
std::string image_name;
std::string snap_name;
int r = utils::get_pool_image_snapshot_names(
vm, at::ARGUMENT_MODIFIER_SOURCE, &arg_index, &pool_name, &namespace_name,
&image_name, nullptr, true, utils::SNAPSHOT_PRESENCE_NONE,
&image_name, import_only ? &snap_name : nullptr, true,
import_only ? utils::SNAPSHOT_PRESENCE_PERMITTED :
utils::SNAPSHOT_PRESENCE_NONE,
utils::SPEC_VALIDATION_NONE);
if (r < 0) {
return r;
@ -169,7 +175,6 @@ int execute_prepare(const po::variables_map &vm,
return r;
}
bool import_only = vm["import-only"].as<bool>();
std::string source_spec;
if (vm.count("source-spec") && vm.count("source-spec-path")) {
std::cerr << "rbd: cannot specify both source-image-spec and "
@ -223,12 +228,18 @@ int execute_prepare(const po::variables_map &vm,
}
if (import_only && source_spec.empty()) {
if (snap_name.empty()) {
std::cerr << "rbd: snapshot name was not specified" << std::endl;
return -EINVAL;
}
std::stringstream ss;
ss << R"({)"
<< R"("type":"native",)"
<< R"("pool_id":)" << io_ctx.get_id() << R"(,)"
<< R"("pool_namespace":")" << io_ctx.get_namespace() << R"(",)"
<< R"("image_name":")" << image_name << R"(")"
<< R"("image_name":")" << image_name << R"(",)"
<< R"("snap_name":")" << snap_name << R"(")"
<< R"(})";
source_spec = ss.str();
@ -238,12 +249,19 @@ int execute_prepare(const po::variables_map &vm,
}
io_ctx = dst_io_ctx;
image_name = dst_image_name;
snap_name = "";
} else if (!import_only && !source_spec.empty()) {
std::cerr << "rbd: --import-only must be used in combination with "
<< "source-spec/source-spec-path" << std::endl;
return -EINVAL;
}
if (!snap_name.empty()) {
std::cerr << "rbd: snapshot name specified for a command that doesn't "
<< "use it" << std::endl;
return -EINVAL;
}
librbd::ImageOptions opts;
r = utils::get_image_options(vm, true, &opts);
if (r < 0) {