On Thu, 27 Dec 2012, Alex Elder wrote: > On 12/27/2012 06:07 PM, Sage Weil wrote: > > We should not set con->state to CLOSED here; that happens in ceph_fault() > > in the caller, where it first asserts that the state is not yet CLOSED. > > I'm not seeing the code path you're talking about. > Do you mean in the LOSSYTX case? It's when the feature bits don't match. It calls this, sets CLOSED< and then returns -1 and con_work() calls ceph_fault(). > > (I don't doubt you're right, I'm just trying to follow > along at home.) > > > Avoids a BUG when the features don't match. > > Is this related to the crashes reported here? > http://tracker.newdream.net/issues/3657 > > > Signed-off-by: Sage Weil <sage@xxxxxxxxxxx> > > --- > > net/ceph/messenger.c | 2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > > > diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c > > index 4d111fd..24a5c86 100644 > > --- a/net/ceph/messenger.c > > +++ b/net/ceph/messenger.c > > @@ -1508,9 +1508,9 @@ static int process_banner(struct ceph_connection *con) > > > > static void fail_protocol(struct ceph_connection *con) > > { > > + dout("fail_protocol %p\n", con); > > reset_connection(con); > > BUG_ON(con->state != CON_STATE_NEGOTIATING); > > Since fail_protocol becomes essentially a trivial wrapper > for reset_connection(), I think it should just go away > and call reset_connection() directly. The assertion that > it's in NEGOTIATING state is not very useful at the moment; > fail_protocol() is only called from process_connect(), > and that's only called from try_read when the state > is NEGOTIATING. Good point. I'll clean that up! > > -Alex > > > - con->state = CON_STATE_CLOSED; > > } > > > > static int process_connect(struct ceph_connection *con) > > > > -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
