Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

librbd: allow to open an image without opening the parent image #12885

Merged
merged 1 commit into from Jan 17, 2017
Merged

librbd: allow to open an image without opening the parent image #12885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 8 additions & 6 deletions src/librbd/ImageState.cc
View file
Open in desktop
Expand Up @@ -234,7 +234,8 @@ ImageState<I>::ImageState(I *image_ctx)
: m_image_ctx(image_ctx), m_state(STATE_UNINITIALIZED),
m_lock(util::unique_lock_name("librbd::ImageState::m_lock", this)),
m_last_refresh(0), m_refresh_seq(0),
m_update_watchers(new ImageUpdateWatchers(image_ctx->cct)) {
m_update_watchers(new ImageUpdateWatchers(image_ctx->cct)),
m_skip_open_parent_image(false) {
}

template <typename I>
Expand All @@ -244,19 +245,20 @@ ImageState<I>::~ImageState() {
}

template <typename I>
int ImageState<I>::open() {
int ImageState<I>::open(bool skip_open_parent) {
C_SaferCond ctx;
open(&ctx);
open(skip_open_parent, &ctx);
return ctx.wait();
}

template <typename I>
void ImageState<I>::open(Context *on_finish) {
void ImageState<I>::open(bool skip_open_parent, Context *on_finish) {
CephContext *cct = m_image_ctx->cct;
ldout(cct, 20) << __func__ << dendl;

m_lock.Lock();
assert(m_state == STATE_UNINITIALIZED);
m_skip_open_parent_image = skip_open_parent;

Action action(ACTION_TYPE_OPEN);
action.refresh_seq = m_refresh_seq;
Expand Down Expand Up @@ -576,7 +578,7 @@ void ImageState<I>::send_open_unlock() {
*m_image_ctx, create_context_callback<
ImageState<I>, &ImageState<I>::handle_open>(this));
image::OpenRequest<I> *req = image::OpenRequest<I>::create(
m_image_ctx, ctx);
m_image_ctx, m_skip_open_parent_image, ctx);

m_lock.Unlock();
req->send();
Expand Down Expand Up @@ -641,7 +643,7 @@ void ImageState<I>::send_refresh_unlock() {
*m_image_ctx, create_context_callback<
ImageState<I>, &ImageState<I>::handle_refresh>(this));
image::RefreshRequest<I> *req = image::RefreshRequest<I>::create(
*m_image_ctx, false, ctx);
*m_image_ctx, false, false, ctx);

m_lock.Unlock();
req->send();
Expand Down
6 changes: 4 additions & 2 deletions src/librbd/ImageState.h
View file
Open in desktop
Expand Up @@ -25,8 +25,8 @@ class ImageState {
ImageState(ImageCtxT *image_ctx);
~ImageState();

int open();
void open(Context *on_finish);
int open(bool skip_open_parent);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: can you just combine these methods and fix the callsites that now need to pass "false"?

void open(bool skip_open_parent, Context *on_finish);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: here as well


int close();
void close(Context *on_finish);
Expand Down Expand Up @@ -109,6 +109,8 @@ class ImageState {

ImageUpdateWatchers *m_update_watchers;

bool m_skip_open_parent_image;

bool is_transition_state() const;
bool is_closed() const;

Expand Down
2 changes: 1 addition & 1 deletion src/librbd/exclusive_lock/PostAcquireRequest.cc
View file
Open in desktop
Expand Up @@ -78,7 +78,7 @@ void PostAcquireRequest<I>::send_refresh() {
// ImageState is blocked waiting for lock to complete -- safe to directly
// refresh
image::RefreshRequest<I> *req = image::RefreshRequest<I>::create(
m_image_ctx, true, ctx);
m_image_ctx, true, false, ctx);
req->send();
}

Expand Down
8 changes: 5 additions & 3 deletions src/librbd/image/OpenRequest.cc
View file
Open in desktop
Expand Up @@ -30,8 +30,10 @@ using util::create_context_callback;
using util::create_rados_ack_callback;

template <typename I>
OpenRequest<I>::OpenRequest(I *image_ctx, Context *on_finish)
: m_image_ctx(image_ctx), m_on_finish(on_finish), m_error_result(0),
OpenRequest<I>::OpenRequest(I *image_ctx, bool skip_open_parent,
Context *on_finish)
: m_image_ctx(image_ctx), m_skip_open_parent_image(skip_open_parent),
m_on_finish(on_finish), m_error_result(0),
m_last_metadata_key(ImageCtx::METADATA_CONF_PREFIX) {
}

Expand Down Expand Up @@ -424,7 +426,7 @@ void OpenRequest<I>::send_refresh() {

using klass = OpenRequest<I>;
RefreshRequest<I> *req = RefreshRequest<I>::create(
*m_image_ctx, false,
*m_image_ctx, false, m_skip_open_parent_image,
create_context_callback<klass, &klass::handle_refresh>(this));
req->send();
}
Expand Down
8 changes: 5 additions & 3 deletions src/librbd/image/OpenRequest.h
View file
Open in desktop
Expand Up @@ -19,8 +19,9 @@ namespace image {
template <typename ImageCtxT = ImageCtx>
class OpenRequest {
public:
static OpenRequest *create(ImageCtxT *image_ctx, Context *on_finish) {
return new OpenRequest(image_ctx, on_finish);
static OpenRequest *create(ImageCtxT *image_ctx, bool skip_open_parent,
Context *on_finish) {
return new OpenRequest(image_ctx, skip_open_parent, on_finish);
}

void send();
Expand Down Expand Up @@ -68,9 +69,10 @@ class OpenRequest {
* @endverbatim
*/

OpenRequest(ImageCtxT *image_ctx, Context *on_finish);
OpenRequest(ImageCtxT *image_ctx, bool skip_open_parent, Context *on_finish);

ImageCtxT *m_image_ctx;
bool m_skip_open_parent_image;
Context *m_on_finish;

bufferlist m_out_bl;
Expand Down
2 changes: 1 addition & 1 deletion src/librbd/image/RefreshParentRequest.cc
View file
Open in desktop
Expand Up @@ -120,7 +120,7 @@ void RefreshParentRequest<I>::send_open_parent() {
Context *ctx = create_async_context_callback(
m_child_image_ctx, create_context_callback<
klass, &klass::handle_open_parent, false>(this));
OpenRequest<I> *req = OpenRequest<I>::create(m_parent_image_ctx, ctx);
OpenRequest<I> *req = OpenRequest<I>::create(m_parent_image_ctx, false, ctx);
req->send();
}

Expand Down
7 changes: 4 additions & 3 deletions src/librbd/image/RefreshRequest.cc
View file
Open in desktop
Expand Up @@ -28,8 +28,9 @@ using util::create_context_callback;

template <typename I>
RefreshRequest<I>::RefreshRequest(I &image_ctx, bool acquiring_lock,
Context *on_finish)
bool skip_open_parent, Context *on_finish)
: m_image_ctx(image_ctx), m_acquiring_lock(acquiring_lock),
m_skip_open_parent_image(skip_open_parent),
m_on_finish(create_async_context_callback(m_image_ctx, on_finish)),
m_error_result(0), m_flush_aio(false), m_exclusive_lock(nullptr),
m_object_map(nullptr), m_journal(nullptr), m_refresh_parent(nullptr) {
Expand Down Expand Up @@ -496,8 +497,8 @@ void RefreshRequest<I>::send_v2_refresh_parent() {

parent_info parent_md;
int r = get_parent_info(m_image_ctx.snap_id, &parent_md);
if (r < 0 ||
RefreshParentRequest<I>::is_refresh_required(m_image_ctx, parent_md)) {
if (!m_skip_open_parent_image && (r < 0 ||
RefreshParentRequest<I>::is_refresh_required(m_image_ctx, parent_md))) {
CephContext *cct = m_image_ctx.cct;
ldout(cct, 10) << this << " " << __func__ << dendl;

Expand Down
9 changes: 6 additions & 3 deletions src/librbd/image/RefreshRequest.h
View file
Open in desktop
Expand Up @@ -27,11 +27,13 @@ template<typename ImageCtxT = ImageCtx>
class RefreshRequest {
public:
static RefreshRequest *create(ImageCtxT &image_ctx, bool acquiring_lock,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: same comment here re: only having one factory method

Context *on_finish) {
return new RefreshRequest(image_ctx, acquiring_lock, on_finish);
bool skip_open_parent, Context *on_finish) {
return new RefreshRequest(image_ctx, acquiring_lock, skip_open_parent,
on_finish);
}

RefreshRequest(ImageCtxT &image_ctx, bool acquiring_lock, Context *on_finish);
RefreshRequest(ImageCtxT &image_ctx, bool acquiring_lock,
bool skip_open_parent, Context *on_finish);
~RefreshRequest();

void send();
Expand Down Expand Up @@ -104,6 +106,7 @@ class RefreshRequest {

ImageCtxT &m_image_ctx;
bool m_acquiring_lock;
bool m_skip_open_parent_image;
Context *m_on_finish;

int m_error_result;
Expand Down
16 changes: 8 additions & 8 deletions src/librbd/internal.cc
View file
Open in desktop
Expand Up @@ -710,7 +710,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx,

for (auto &id_it : info.second) {
ImageCtx *imctx = new ImageCtx("", id_it, NULL, ioctx, false);
int r = imctx->state->open();
int r = imctx->state->open(false);
if (r < 0) {
lderr(cct) << "error opening image: "
<< cpp_strerror(r) << dendl;
Expand Down Expand Up @@ -1082,7 +1082,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx,

// make sure parent snapshot exists
ImageCtx *p_imctx = new ImageCtx(p_name, "", p_snap_name, p_ioctx, true);
int r = p_imctx->state->open();
int r = p_imctx->state->open(false);
if (r < 0) {
lderr(cct) << "error opening parent image: "
<< cpp_strerror(r) << dendl;
Expand Down Expand Up @@ -1224,7 +1224,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx,
}

c_imctx = new ImageCtx(c_name, "", NULL, c_ioctx, false);
r = c_imctx->state->open();
r = c_imctx->state->open(false);
if (r < 0) {
lderr(cct) << "Error opening new image: " << cpp_strerror(r) << dendl;
delete c_imctx;
Expand Down Expand Up @@ -1326,7 +1326,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx,
<< dstname << dendl;

ImageCtx *ictx = new ImageCtx(srcname, "", "", io_ctx, false);
int r = ictx->state->open();
int r = ictx->state->open(false);
if (r < 0) {
lderr(ictx->cct) << "error opening source image: " << cpp_strerror(r)
<< dendl;
Expand Down Expand Up @@ -1649,7 +1649,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx,
bool unknown_format = true;
ImageCtx *ictx = new ImageCtx(
(id.empty() ? name : std::string()), id, nullptr, io_ctx, false);
int r = ictx->state->open();
int r = ictx->state->open(true);
if (r < 0) {
ldout(cct, 2) << "error opening image: " << cpp_strerror(-r) << dendl;
delete ictx;
Expand Down Expand Up @@ -2011,7 +2011,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx,

ImageCtx *dest = new librbd::ImageCtx(destname, "", NULL,
dest_md_ctx, false);
r = dest->state->open();
r = dest->state->open(false);
if (r < 0) {
delete dest;
lderr(cct) << "failed to read newly created header" << dendl;
Expand Down Expand Up @@ -3050,7 +3050,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx,
if ((features & RBD_FEATURE_JOURNALING) != 0) {
ImageCtx *img_ctx = new ImageCtx("", img_pair.second, nullptr,
io_ctx, false);
r = img_ctx->state->open();
r = img_ctx->state->open(false);
if (r < 0) {
lderr(cct) << "error opening image "<< img_pair.first << ": "
<< cpp_strerror(r) << dendl;
Expand Down Expand Up @@ -3098,7 +3098,7 @@ void filter_out_mirror_watchers(ImageCtx *ictx,
}
} else {
ImageCtx *img_ctx = new ImageCtx("", img_id, nullptr, io_ctx, false);
r = img_ctx->state->open();
r = img_ctx->state->open(false);
if (r < 0) {
lderr(cct) << "error opening image id "<< img_id << ": "
<< cpp_strerror(r) << dendl;
Expand Down
21 changes: 11 additions & 10 deletions src/librbd/librbd.cc
View file
Open in desktop
Expand Up @@ -106,7 +106,7 @@ struct C_OpenAfterCloseComplete : public Context {
virtual void finish(int r) {
ldout(ictx->cct, 20) << "C_OpenAfterCloseComplete::finish: r=" << r
<< dendl;
ictx->state->open(new C_OpenComplete(ictx, comp, ictxp, true));
ictx->state->open(false, new C_OpenComplete(ictx, comp, ictxp, true));
}
};

Expand Down Expand Up @@ -229,7 +229,7 @@ namespace librbd {
image.ctx = NULL;
}

int r = ictx->state->open();
int r = ictx->state->open(false);
if (r < 0) {
delete ictx;
tracepoint(librbd, open_image_exit, r);
Expand All @@ -252,7 +252,7 @@ namespace librbd {
reinterpret_cast<ImageCtx*>(image.ctx)->state->close(
new C_OpenAfterCloseComplete(ictx, get_aio_completion(c), &image.ctx));
} else {
ictx->state->open(new C_OpenComplete(ictx, get_aio_completion(c),
ictx->state->open(false, new C_OpenComplete(ictx, get_aio_completion(c),
&image.ctx));
}
tracepoint(librbd, aio_open_image_exit, 0);
Expand All @@ -271,7 +271,7 @@ namespace librbd {
image.ctx = NULL;
}

int r = ictx->state->open();
int r = ictx->state->open(false);
if (r < 0) {
delete ictx;
tracepoint(librbd, open_image_exit, r);
Expand All @@ -294,7 +294,7 @@ namespace librbd {
reinterpret_cast<ImageCtx*>(image.ctx)->state->close(
new C_OpenAfterCloseComplete(ictx, get_aio_completion(c), &image.ctx));
} else {
ictx->state->open(new C_OpenComplete(ictx, get_aio_completion(c),
ictx->state->open(false, new C_OpenComplete(ictx, get_aio_completion(c),
&image.ctx));
}
tracepoint(librbd, aio_open_image_exit, 0);
Expand Down Expand Up @@ -2050,7 +2050,7 @@ extern "C" int rbd_open(rados_ioctx_t p, const char *name, rbd_image_t *image,
false);
tracepoint(librbd, open_image_enter, ictx, ictx->name.c_str(), ictx->id.c_str(), ictx->snap_name.c_str(), ictx->read_only);

int r = ictx->state->open();
int r = ictx->state->open(false);
if (r < 0) {
delete ictx;
} else {
Expand All @@ -2071,7 +2071,7 @@ extern "C" int rbd_aio_open(rados_ioctx_t p, const char *name,
false);
librbd::RBD::AioCompletion *comp = (librbd::RBD::AioCompletion *)c;
tracepoint(librbd, aio_open_image_enter, ictx, ictx->name.c_str(), ictx->id.c_str(), ictx->snap_name.c_str(), ictx->read_only, comp->pc);
ictx->state->open(new C_OpenComplete(ictx, get_aio_completion(comp), image));
ictx->state->open(false, new C_OpenComplete(ictx, get_aio_completion(comp), image));
tracepoint(librbd, aio_open_image_exit, 0);
return 0;
}
Expand All @@ -2086,7 +2086,7 @@ extern "C" int rbd_open_read_only(rados_ioctx_t p, const char *name,
true);
tracepoint(librbd, open_image_enter, ictx, ictx->name.c_str(), ictx->id.c_str(), ictx->snap_name.c_str(), ictx->read_only);

int r = ictx->state->open();
int r = ictx->state->open(false);
if (r < 0) {
delete ictx;
} else {
Expand All @@ -2107,7 +2107,8 @@ extern "C" int rbd_aio_open_read_only(rados_ioctx_t p, const char *name,
true);
librbd::RBD::AioCompletion *comp = (librbd::RBD::AioCompletion *)c;
tracepoint(librbd, aio_open_image_enter, ictx, ictx->name.c_str(), ictx->id.c_str(), ictx->snap_name.c_str(), ictx->read_only, comp->pc);
ictx->state->open(new C_OpenComplete(ictx, get_aio_completion(comp), image));
ictx->state->open(false, new C_OpenComplete(ictx, get_aio_completion(comp),
image));
tracepoint(librbd, aio_open_image_exit, 0);
return 0;
}
Expand Down Expand Up @@ -3332,7 +3333,7 @@ extern "C" int rbd_image_get_group(rados_ioctx_t image_p,
librados::IoCtx::from_rados_ioctx_t(image_p, io_ctx);

librbd::ImageCtx *ictx = new librbd::ImageCtx(image_name, "", "", io_ctx, false);
int r = ictx->state->open();
int r = ictx->state->open(false);
if (r < 0) {
delete ictx;
tracepoint(librbd, open_image_exit, r);
Expand Down
2 changes: 1 addition & 1 deletion src/test/librbd/exclusive_lock/test_mock_PostAcquireRequest.cc
View file
Open in desktop
Expand Up @@ -41,7 +41,7 @@ struct RefreshRequest<librbd::MockTestImageCtx> {

static RefreshRequest *create(librbd::MockTestImageCtx &image_ctx,
bool acquire_lock_refresh,
Context *on_finish) {
bool skip_open_parent, Context *on_finish) {
EXPECT_TRUE(acquire_lock_refresh);
assert(s_instance != nullptr);
s_instance->on_finish = on_finish;
Expand Down