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.

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

rbd: add group rename methods #20577

Merged
merged 3 commits into from Feb 27, 2018

Conversation

Projects
None yet
4 participants
@Songweibin
Copy link
Contributor

commented Feb 24, 2018

@xiexingguo xiexingguo requested a review from dillaman Feb 24, 2018

@@ -159,4 +159,4 @@ Task to do:
details to figure out

- Design a vitual disk implementation that can be used with behyve and
attached to an RBD image.
attached to a RBD image.

This comment has been minimized.

Copy link
@dillaman

dillaman Feb 25, 2018

Contributor

These are actually correct -- an should be used instead of a since the "R" sound in the acronym "RBD" starts with an /a/ sound.

This comment has been minimized.

Copy link
@dillaman

dillaman Feb 25, 2018

Contributor

... and it's general advisable to not combine unrelated commits within a single PR.

@@ -5107,6 +5107,86 @@ int mirror_image_map_remove(cls_method_context_t hctx, bufferlist *in,
return 0;
}

/*********************** methods for rbd_group_directory ***********************/

static int group_dir_add_helper(cls_method_context_t hctx,

This comment has been minimized.

Copy link
@dillaman

dillaman Feb 25, 2018

Contributor

Nit: no need for static nor group_ prefix when you put these helper methods in the existing group namespace. Also, the suffix of helper is not very descriptive as to what the purpose of this function is -- I think you can just use int dir_add within the namespace.


CLS_LOG(20, "group_dir_add name=%s id=%s", name.c_str(), id.c_str());

string tmp;

This comment has been minimized.

Copy link
@dillaman

dillaman Feb 25, 2018

Contributor

Nit: declare right before first use

return cls_cxx_map_set_vals(hctx, &omap_vals);
}

static int group_dir_remove_helper(cls_method_context_t hctx,

This comment has been minimized.

Copy link
@dillaman

dillaman Feb 25, 2018

Contributor

Nit: same comment here: int dir_remove()

@Songweibin Songweibin force-pushed the Songweibin:wip-group-rename branch from 5b06e31 to 4b1f9b5 Feb 26, 2018

Songweibin added some commits Feb 24, 2018

cls_rbd: add group rename operations to cls_rbd
Signed-off-by: songweibin <song.weibin@zte.com.cn>
librbd: add group rename methods
Signed-off-by: songweibin <song.weibin@zte.com.cn>
rbd: add 'group rename' action to CLI and Python API
Signed-off-by: songweibin <song.weibin@zte.com.cn>

@Songweibin Songweibin force-pushed the Songweibin:wip-group-rename branch from 4b1f9b5 to 034ad87 Feb 26, 2018

@Songweibin

This comment has been minimized.

Copy link
Contributor Author

commented Feb 26, 2018

@dillaman Updated, thanks.

@dillaman
Copy link
Contributor

left a comment

lgtm

@@ -5107,6 +5107,86 @@ int mirror_image_map_remove(cls_method_context_t hctx, bufferlist *in,
return 0;
}

/*********************** methods for rbd_group_directory ***********************/

int dir_add(cls_method_context_t hctx,

This comment has been minimized.

Copy link
@trociny

trociny Feb 27, 2018

Contributor

@dillaman Didn't you wanted this in "group" namespace?

@dillaman

This comment has been minimized.

Copy link
Contributor

commented Feb 27, 2018

Manually merged after correcting group namespace issue

@dillaman dillaman closed this Feb 27, 2018

@dillaman dillaman merged commit 034ad87 into ceph:master Feb 27, 2018

5 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details

dillaman added a commit that referenced this pull request Feb 27, 2018

Merge pull request #20577 from Songweibin/wip-group-rename
rbd: add group rename methods

Reviewed-by: Jason Dillaman <dillaman@redhat.com>

@Songweibin Songweibin deleted the Songweibin:wip-group-rename branch Feb 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.