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

mimic: osd/OSDMap: Replace get_out_osds with get_out_existing_osds #28142

Merged
merged 1 commit into from Jul 17, 2019

Conversation

@pdvian
Copy link

pdvian commented May 16, 2019

@@ -699,7 +699,7 @@ class OSDMap {

void get_all_osds(set<int32_t>& ls) const;
void get_up_osds(set<int32_t>& ls) const;
void get_out_osds(set<int32_t>& ls) const;
void get_out_existing_osds(std::set<int32_t>& ls) const;

This comment has been minimized.

Copy link
@pdvian

pdvian May 16, 2019

Author

@badone Should we keep uniform set<int32_t> instead of std::set<int32_t> ?

This comment has been minimized.

Copy link
@badone

badone May 19, 2019

Contributor

@pdvian Yes. I would keep it uniform even though the fully qualified form is more desirable. If you want to create a seperate PR to qualify the std::set references that would be cool. Check out the PR that fully qualified these in master and let me know if I can help.

This comment has been minimized.

Copy link
@pdvian

pdvian Jun 5, 2019

Author

@badone the commit-id a192c1a has above fully qualified form for these functions. The PR#27255 is quite big and need separate backport PR though we donot have tracker for 27255. Let me know your thoughts.

This comment has been minimized.

Copy link
@neha-ojha

neha-ojha Jun 6, 2019

Member

@badone is out, let's go ahead without the std:: for this PR.

This comment has been minimized.

Copy link
@pdvian

pdvian Jun 11, 2019

Author

Sure. Let me do the changes accordingly.

@smithfarm smithfarm added this to the mimic milestone May 17, 2019
@smithfarm smithfarm requested a review from badone May 17, 2019
@smithfarm smithfarm added the core label May 17, 2019
@smithfarm smithfarm requested a review from neha-ojha May 17, 2019
@badone
badone approved these changes May 19, 2019
Copy link
Contributor

badone left a comment

LGTM once inconsistencies mentioned above are resolved.

@neha-ojha

This comment has been minimized.

Copy link
Member

neha-ojha commented Jun 3, 2019

LGTM once inconsistencies mentioned above are resolved.

looks like your comment hasn't been addressed, which is holding up this PR from moving to "needs-qa"?

@pdvian

This comment has been minimized.

Copy link
Author

pdvian commented Jun 5, 2019

@badone Let me check if we have PR in master for above fully qualified form in OSDMap. @neha-ojha Thanks for ping.

@pdvian

This comment has been minimized.

Copy link
Author

pdvian commented Jun 6, 2019

Waiting for @badone inputs.

@pdvian pdvian force-pushed the pdvian:wip-39422-mimic branch from 9c209e6 to c596e8f Jun 11, 2019
Fixes: http://tracker.ceph.com/issues/39154

Signed-off-by: Brad Hubbard <bhubbard@redhat.com>
(cherry picked from commit adfb6a5)

Conflicts:
	src/osd/OSDMap.h : Resovled for get_out_existing_osds
@pdvian pdvian force-pushed the pdvian:wip-39422-mimic branch from c596e8f to a2353be Jun 11, 2019
@badone

This comment has been minimized.

Copy link
Contributor

badone commented Jun 17, 2019

LGTM (was on holiday)

@yuriw

This comment has been minimized.

Copy link
Contributor

yuriw commented Jul 8, 2019

@yuriw yuriw merged commit 016888d into ceph:mimic Jul 17, 2019
4 checks passed
4 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.