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.

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

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

Merged
merged 1 commit into from Jan 17, 2017

Conversation

Projects
None yet
2 participants
@rjfd
Contributor

rjfd commented Jan 11, 2017

Fixes: http://tracker.ceph.com/issues/18325

Signed-off-by: Ricardo Dias rdias@suse.com

@rjfd rjfd added bug fix rbd labels Jan 11, 2017

@dillaman dillaman changed the title from rbd: allow to open an image without opening the parent image to librbd: allow to open an image without opening the parent image Jan 11, 2017

@@ -25,8 +25,8 @@ class ImageState {
ImageState(ImageCtxT *image_ctx);
~ImageState();
int open();
void open(Context *on_finish);
int open(bool dont_open_parent=false);

This comment has been minimized.

@dillaman

dillaman Jan 13, 2017

Contributor

Nit: can we rename this to something like skip_open_parent everywhere? Something about "dont" feels a bit off

@@ -25,8 +25,8 @@ class ImageState {
ImageState(ImageCtxT *image_ctx);
~ImageState();
int open();
void open(Context *on_finish);
int open(bool dont_open_parent=false);

This comment has been minimized.

@dillaman

dillaman Jan 13, 2017

Contributor

Nit: avoid defaulting the parameter here and below

int open();
void open(Context *on_finish);
int open(bool dont_open_parent=false);
void open(Context *on_finish, bool dont_open_parent=false);

This comment has been minimized.

@dillaman

dillaman Jan 13, 2017

Contributor

Nit: can you move on_finish to be the last param?

@@ -20,7 +20,12 @@ template <typename ImageCtxT = ImageCtx>
class OpenRequest {
public:
static OpenRequest *create(ImageCtxT *image_ctx, Context *on_finish) {

This comment has been minimized.

@dillaman

dillaman Jan 13, 2017

Contributor

Nit: only need one factory method -- remove this one and update where necessary

@@ -28,10 +28,17 @@ class RefreshRequest {
public:
static RefreshRequest *create(ImageCtxT &image_ctx, bool acquiring_lock,

This comment has been minimized.

@dillaman

dillaman Jan 13, 2017

Contributor

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

@rjfd

This comment has been minimized.

Contributor

rjfd commented Jan 16, 2017

@dillaman updated PR with your comments

@rjfd

This comment has been minimized.

Contributor

rjfd commented Jan 16, 2017

jenkins retest this please

@@ -26,7 +26,9 @@ class ImageState {
~ImageState();
int open();
int open(bool skip_open_parent);

This comment has been minimized.

@dillaman

dillaman Jan 16, 2017

Contributor

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

@rjfd

This comment has been minimized.

Contributor

rjfd commented Jan 16, 2017

@dillaman pushed new changes

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

This comment has been minimized.

@dillaman

dillaman Jan 16, 2017

Contributor

Nit: here as well

@rjfd

This comment has been minimized.

Contributor

rjfd commented Jan 16, 2017

@dillaman done

@dillaman dillaman merged commit 449d783 into ceph:master Jan 17, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details

@rjfd rjfd deleted the rjfd:wip-18325 branch Jan 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment