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

mon/OSDMonitor: should propose osdmap update when cluster addr changed #11065

Merged
merged 1 commit into from Dec 24, 2016

Conversation

Projects
None yet
4 participants
@LiumxNL
Contributor

LiumxNL commented Sep 13, 2016

osd maybe rebind cluster addr but keep public addr unchanged in _committed_osd_maps() method, to be on the safe side, monitor should take cluster addr into account when determining whether should propose osdmap update.

Signed-off-by: Mingxin Liu mingxin@xsky.com

@LiumxNL

This comment has been minimized.

Contributor

LiumxNL commented Sep 13, 2016

@tchaikov would you mind to take a look? thanks!

@tchaikov tchaikov self-assigned this Sep 13, 2016

@@ -2108,7 +2109,9 @@ bool OSDMonitor::prepare_boot(MonOpRequestRef op)
if (osdmap.is_up(from)) {
dout(7) << "prepare_boot was up, first marking down " << osdmap.get_inst(from) << dendl;
// preprocess should have caught these; if not, assert.
assert(osdmap.get_inst(from) != m->get_orig_source_inst());
bool addr_changed = (osdmap.get_inst(from) != m->get_orig_source_inst() ||

This comment has been minimized.

@tchaikov

tchaikov Sep 19, 2016

Contributor

could also put osdmap.get_cluster_addr(from) != m->cluster_addr) on a new line. like

assert(osdmap.get_inst(from) != m->get_orig_source_inst() ||
       osdmap.get_cluster_addr(from) != m->cluster_addr);

as ideally, assert() is a no-op in a NDEBUG build, and compiler or static analyzer would complain at seeing a variable not being used anywhere.

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Sep 19, 2016

lgtm modulo the nit.

@tchaikov tchaikov assigned LiumxNL and unassigned tchaikov Sep 21, 2016

@yuyuyu101

This comment has been minimized.

Member

yuyuyu101 commented Sep 29, 2016

@LiumxNL ping

@ghost ghost added core bug fix labels Nov 23, 2016

OSDMonitor: should propose osdmap update when OSD cluster_addr was ch…
…anged

Signed-off-by: Mingxin Liu <mingxin@xsky.com>
@LiumxNL

This comment has been minimized.

Contributor

LiumxNL commented Dec 22, 2016

@tchaikov tchaikov added the needs-qa label Dec 22, 2016

@tchaikov tchaikov assigned tchaikov and unassigned LiumxNL Dec 22, 2016

@liewegas liewegas changed the title from OSDMonitor: should propose osdmap update when cluster addr changed to mon/OSDMonitor: should propose osdmap update when cluster addr changed Dec 23, 2016

@liewegas liewegas merged commit 20a1f3d into ceph:master Dec 24, 2016

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment