Skip to content

Commit

Permalink
Merge pull request #23733 from dillaman/wip-migration-parent
Browse files Browse the repository at this point in the history 
librbd: always open first parent image if it exists for a snapshot

Reviewed-by: Mykola Golub <mgolub@suse.com>
  • Loading branch information
@trociny
trociny committed Aug 29, 2018
2 parents bd82967 + bed4857 commit 9f9b525
Show file tree
Hide file tree
Showing 14 changed files with 69 additions and 342 deletions.
1 change: 0 additions & 1 deletion src/librbd/ImageCtx.h
View file
Expand Up @@ -125,7 +125,6 @@ namespace librbd {
ImageCtx *parent;
ImageCtx *child = nullptr;
MigrationInfo migration_info;
ImageCtx *migration_parent = nullptr;
cls::rbd::GroupSpec group_spec;
uint64_t stripe_unit, stripe_count;
uint64_t flags;
Expand Down
4 changes: 2 additions & 2 deletions src/librbd/Types.h
View file
Expand Up @@ -117,8 +117,8 @@ struct SnapInfo {

enum {
OPEN_FLAG_SKIP_OPEN_PARENT = 1 << 0,
OPEN_FLAG_OLD_FORMAT = 1 << 1,
OPEN_FLAG_IGNORE_MIGRATING = 1 << 2,
OPEN_FLAG_OLD_FORMAT = 1 << 1,
OPEN_FLAG_IGNORE_MIGRATING = 1 << 2
};

struct MigrationInfo {
Expand Down
102 changes: 3 additions & 99 deletions src/librbd/deep_copy/ImageCopyRequest.cc
View file
Expand Up @@ -48,7 +48,7 @@ void ImageCopyRequest<I>::send() {
return;
}

send_open_parent();
send_object_copies();
}

template <typename I>
Expand All @@ -59,69 +59,6 @@ void ImageCopyRequest<I>::cancel() {
m_canceled = true;
}

template <typename I>
void ImageCopyRequest<I>::send_open_parent() {
ParentSpec parent_spec;
{
RWLock::RLocker snap_locker(m_src_image_ctx->snap_lock);
RWLock::RLocker parent_locker(m_src_image_ctx->parent_lock);

auto snap_id = m_snap_map.begin()->first;
auto parent_info = m_src_image_ctx->get_parent_info(snap_id);
if (parent_info == nullptr) {
ldout(m_cct, 20) << "could not find parent info for snap id " << snap_id
<< dendl;
} else {
parent_spec = parent_info->spec;
}
}

if (parent_spec.pool_id == -1) {
send_object_copies();
return;
}

ldout(m_cct, 20) << "pool_id=" << parent_spec.pool_id << ", image_id="
<< parent_spec.image_id << ", snap_id="
<< parent_spec.snap_id << dendl;

librados::Rados rados(m_src_image_ctx->md_ctx);
librados::IoCtx parent_io_ctx;
int r = rados.ioctx_create2(parent_spec.pool_id, parent_io_ctx);
if (r < 0) {
lderr(m_cct) << "failed to access parent pool (id=" << parent_spec.pool_id
<< "): " << cpp_strerror(r) << dendl;
finish(r);
return;
}

// TODO support clone v2 parent namespaces
parent_io_ctx.set_namespace(m_src_image_ctx->md_ctx.get_namespace());

m_src_parent_image_ctx = I::create("", parent_spec.image_id,
parent_spec.snap_id, parent_io_ctx, true);
auto ctx = create_context_callback<
ImageCopyRequest<I>, &ImageCopyRequest<I>::handle_open_parent>(this);

auto req = image::OpenRequest<I>::create(m_src_parent_image_ctx, false, ctx);
req->send();
}

template <typename I>
void ImageCopyRequest<I>::handle_open_parent(int r) {
ldout(m_cct, 20) << "r=" << r << dendl;

if (r < 0) {
lderr(m_cct) << "failed to open parent: " << cpp_strerror(r) << dendl;
m_src_parent_image_ctx->destroy();
m_src_parent_image_ctx = nullptr;
finish(r);
return;
}

send_object_copies();
}

template <typename I>
void ImageCopyRequest<I>::send_object_copies() {
m_object_no = 0;
Expand Down Expand Up @@ -157,7 +94,7 @@ void ImageCopyRequest<I>::send_object_copies() {
}

if (complete) {
send_close_parent();
finish(m_ret_val);
}
}

Expand Down Expand Up @@ -185,8 +122,7 @@ void ImageCopyRequest<I>::send_next_object_copy() {
handle_object_copy(ono, r);
});
ObjectCopyRequest<I> *req = ObjectCopyRequest<I>::create(
m_src_image_ctx, m_src_parent_image_ctx, m_dst_image_ctx, m_snap_map, ono,
m_flatten, ctx);
m_src_image_ctx, m_dst_image_ctx, m_snap_map, ono, m_flatten, ctx);
req->send();
}

Expand Down Expand Up @@ -226,40 +162,8 @@ void ImageCopyRequest<I>::handle_object_copy(uint64_t object_no, int r) {
}

if (complete) {
send_close_parent();
}
}

template <typename I>
void ImageCopyRequest<I>::send_close_parent() {
if (m_src_parent_image_ctx == nullptr) {
finish(m_ret_val);
return;
}

ldout(m_cct, 20) << dendl;

auto ctx = create_context_callback<
ImageCopyRequest<I>, &ImageCopyRequest<I>::handle_close_parent>(this);
auto req = image::CloseRequest<I>::create(m_src_parent_image_ctx, ctx);
req->send();
}

template <typename I>
void ImageCopyRequest<I>::handle_close_parent(int r) {
ldout(m_cct, 20) << "r=" << r << dendl;

if (r < 0) {
lderr(m_cct) << "failed to close parent: " << cpp_strerror(r) << dendl;
if (m_ret_val == 0) {
m_ret_val = r;
}
}

m_src_parent_image_ctx->destroy();
m_src_parent_image_ctx = nullptr;

finish(m_ret_val);
}

template <typename I>
Expand Down
14 changes: 0 additions & 14 deletions src/librbd/deep_copy/ImageCopyRequest.h
View file
Expand Up @@ -55,19 +55,12 @@ class ImageCopyRequest : public RefCountedObject {
* @verbatim
*
* <start>
* |
* v
* OPEN_PARENT (skip if not needed)
* |
* | . . . . .
* | . . (parallel execution of
* v v . multiple objects at once)
* COPY_OBJECT . . . .
* |
* v
* CLOSE_PARENT (skip if not needed)
* |
* v
* <finish>
*
* @endverbatim
Expand Down Expand Up @@ -95,18 +88,11 @@ class ImageCopyRequest : public RefCountedObject {
bool m_updating_progress = false;
SnapMap m_snap_map;
int m_ret_val = 0;
ImageCtxT *m_src_parent_image_ctx = nullptr;

void send_open_parent();
void handle_open_parent(int r);

void send_object_copies();
void send_next_object_copy();
void handle_object_copy(uint64_t object_no, int r);

void send_close_parent();
void handle_close_parent(int r);

void finish(int r);
};

Expand Down
46 changes: 28 additions & 18 deletions src/librbd/deep_copy/ObjectCopyRequest.cc
View file
Expand Up @@ -41,13 +41,11 @@ using librbd::util::create_rados_callback;

template <typename I>
ObjectCopyRequest<I>::ObjectCopyRequest(I *src_image_ctx,
I *src_parent_image_ctx,
I *dst_image_ctx,
const SnapMap &snap_map,
uint64_t dst_object_number,
bool flatten, Context *on_finish)
: m_src_image_ctx(src_image_ctx),
m_src_parent_image_ctx(src_parent_image_ctx),
m_dst_image_ctx(dst_image_ctx), m_cct(dst_image_ctx->cct),
m_snap_map(snap_map), m_dst_object_number(dst_object_number),
m_flatten(flatten), m_on_finish(on_finish) {
Expand Down Expand Up @@ -229,27 +227,35 @@ void ObjectCopyRequest<I>::handle_read_object(int r) {

template <typename I>
void ObjectCopyRequest<I>::send_read_from_parent() {
m_src_image_ctx->snap_lock.get_read();
m_src_image_ctx->parent_lock.get_read();
io::Extents image_extents;
compute_read_from_parent_ops(&image_extents);
m_src_image_ctx->snap_lock.put_read();

if (image_extents.empty()) {
m_src_image_ctx->parent_lock.put_read();
handle_read_from_parent(0);
return;
}

ldout(m_cct, 20) << dendl;

ceph_assert(m_src_parent_image_ctx != nullptr);
ceph_assert(m_src_image_ctx->parent != nullptr);

auto ctx = create_context_callback<
ObjectCopyRequest<I>, &ObjectCopyRequest<I>::handle_read_from_parent>(this);
auto comp = io::AioCompletion::create_and_start(
ctx, util::get_image_ctx(m_src_image_ctx), io::AIO_TYPE_READ);
ctx, util::get_image_ctx(m_src_image_ctx->parent), io::AIO_TYPE_READ);
ldout(m_cct, 20) << "completion " << comp << ", extents " << image_extents
<< dendl;
io::ImageRequest<I>::aio_read(m_src_parent_image_ctx, comp,

auto src_image_ctx = m_src_image_ctx;
io::ImageRequest<I>::aio_read(src_image_ctx->parent, comp,
std::move(image_extents),
io::ReadResult{&m_read_from_parent_data}, 0,
ZTracer::Trace());
src_image_ctx->parent_lock.put_read();
}

template <typename I>
Expand Down Expand Up @@ -564,8 +570,12 @@ void ObjectCopyRequest<I>::compute_read_ops() {
m_read_snaps = {};
m_zero_interval = {};

m_src_image_ctx->parent_lock.get_read();
bool hide_parent = (m_src_image_ctx->parent != nullptr);
m_src_image_ctx->parent_lock.put_read();

librados::snap_t src_copy_point_snap_id = m_snap_map.rbegin()->first;
bool prev_exists = m_src_parent_image_ctx != nullptr;
bool prev_exists = hide_parent;
uint64_t prev_end_size = prev_exists ?
m_src_image_ctx->layout.object_size : 0;
librados::snap_t start_src_snap_id = 0;
Expand All @@ -589,12 +599,10 @@ void ObjectCopyRequest<I>::compute_read_ops() {
exists = true;
end_size = m_src_image_ctx->layout.object_size;
clone_end_snap_id = end_src_snap_id;
}

if (!exists) {
} else if (!exists) {
end_size = 0;
if (end_src_snap_id == m_snap_map.begin()->first &&
m_src_parent_image_ctx != nullptr && m_snap_set.clones.empty()) {
if (hide_parent && end_src_snap_id == m_snap_map.begin()->first &&
m_snap_set.clones.empty()) {
ldout(m_cct, 20) << "no clones for existing object" << dendl;
exists = true;
diff.insert(0, m_src_image_ctx->layout.object_size);
Expand Down Expand Up @@ -686,8 +694,7 @@ void ObjectCopyRequest<I>::compute_read_ops() {

prev_end_size = end_size;
prev_exists = exists;
if (prev_exists && prev_end_size == 0 &&
m_src_parent_image_ctx != nullptr) {
if (hide_parent && prev_exists && prev_end_size == 0) {
// hide parent
prev_end_size = m_src_image_ctx->layout.object_size;
}
Expand All @@ -702,11 +709,14 @@ void ObjectCopyRequest<I>::compute_read_ops() {
template <typename I>
void ObjectCopyRequest<I>::compute_read_from_parent_ops(
io::Extents *parent_image_extents) {
assert(m_src_image_ctx->snap_lock.is_locked());
assert(m_src_image_ctx->parent_lock.is_locked());

m_read_ops = {};
m_zero_interval = {};
parent_image_extents->clear();

if (m_src_parent_image_ctx == nullptr) {
if (m_src_image_ctx->parent == nullptr) {
ldout(m_cct, 20) << "no parent" << dendl;
return;
}
Expand All @@ -733,9 +743,6 @@ void ObjectCopyRequest<I>::compute_read_from_parent_ops(

auto src_snap_seq = m_snap_map.begin()->first;

RWLock::RLocker snap_locker(m_src_image_ctx->snap_lock);
RWLock::RLocker parent_locker(m_src_image_ctx->parent_lock);

uint64_t parent_overlap;
int r = m_src_image_ctx->get_parent_overlap(src_snap_seq, &parent_overlap);
if (r < 0) {
Expand Down Expand Up @@ -835,7 +842,10 @@ void ObjectCopyRequest<I>::compute_zero_ops() {

bool fast_diff = m_dst_image_ctx->test_features(RBD_FEATURE_FAST_DIFF);
uint64_t prev_end_size = 0;
bool hide_parent = m_src_parent_image_ctx != nullptr;

m_src_image_ctx->parent_lock.get_read();
bool hide_parent = (m_src_image_ctx->parent != nullptr);
m_src_image_ctx->parent_lock.put_read();

for (auto &it : m_dst_zero_interval) {
auto src_snap_seq = it.first;
Expand Down
13 changes: 5 additions & 8 deletions src/librbd/deep_copy/ObjectCopyRequest.h
View file
Expand Up @@ -25,19 +25,17 @@ template <typename ImageCtxT = librbd::ImageCtx>
class ObjectCopyRequest {
public:
static ObjectCopyRequest* create(ImageCtxT *src_image_ctx,
ImageCtxT *src_parent_image_ctx,
ImageCtxT *dst_image_ctx,
const SnapMap &snap_map,
uint64_t object_number, bool flatten,
Context *on_finish) {
return new ObjectCopyRequest(src_image_ctx, src_parent_image_ctx,
dst_image_ctx, snap_map, object_number,
flatten, on_finish);
return new ObjectCopyRequest(src_image_ctx, dst_image_ctx, snap_map,
object_number, flatten, on_finish);
}

ObjectCopyRequest(ImageCtxT *src_image_ctx, ImageCtxT *src_parent_image_ctx,
ImageCtxT *dst_image_ctx, const SnapMap &snap_map,
uint64_t object_number, bool flatten, Context *on_finish);
ObjectCopyRequest(ImageCtxT *src_image_ctx, ImageCtxT *dst_image_ctx,
const SnapMap &snap_map, uint64_t object_number,
bool flatten, Context *on_finish);

void send();

Expand Down Expand Up @@ -133,7 +131,6 @@ class ObjectCopyRequest {
typedef std::map<librados::snap_t, std::map<uint64_t, uint64_t>> SnapObjectSizes;

ImageCtxT *m_src_image_ctx;
ImageCtxT *m_src_parent_image_ctx;
ImageCtxT *m_dst_image_ctx;
CephContext *m_cct;
const SnapMap &m_snap_map;
Expand Down
30 changes: 0 additions & 30 deletions src/librbd/image/CloseRequest.cc
View file
Expand Up @@ -266,36 +266,6 @@ template <typename I>
void CloseRequest<I>::handle_flush_op_work_queue(int r) {
CephContext *cct = m_image_ctx->cct;
ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;
send_close_migration_parent();
}

template <typename I>
void CloseRequest<I>::send_close_migration_parent() {
if (m_image_ctx->migration_parent == nullptr) {
send_close_parent();
return;
}

CephContext *cct = m_image_ctx->cct;
ldout(cct, 10) << this << " " << __func__ << dendl;

m_image_ctx->migration_parent->state->close(create_async_context_callback(
*m_image_ctx, create_context_callback<
CloseRequest<I>, &CloseRequest<I>::handle_close_migration_parent>(this)));
}

template <typename I>
void CloseRequest<I>::handle_close_migration_parent(int r) {
CephContext *cct = m_image_ctx->cct;
ldout(cct, 10) << this << " " << __func__ << ": r=" << r << dendl;

delete m_image_ctx->migration_parent;
m_image_ctx->migration_parent = nullptr;
save_result(r);
if (r < 0) {
lderr(cct) << "error closing migration parent image: " << cpp_strerror(r)
<< dendl;
}
send_close_parent();
}

Expand Down

0 comments on commit 9f9b525

Please sign in to comment.