From: Sage Weil <sage@inktank.com>
Date: Wed, 20 Jun 2012 18:07:29 +0000 (-0700)
Subject: objecter: do not feed session to op_submit()
X-Git-Tag: v0.49~83^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ff67210ec2e754c13d7d8bcbf0f01121ee82f722;p=ceph.git

objecter: do not feed session to op_submit()

The linger_send() method was doing this, but it is problematic because the
new Op doesn't get its pgid or acting vector set correctly.  The result is
that the request goes to the right OSD, but has the wrong pgid, and makes
the OSD complain about misdirected requests and drop it on the floor.  It
didn't affect the test results because we weren't testing whether the
watch was working in that case.

Instead, we'll just recalculate and get the same value the parent linger
op did.  Which is fine, and goes through all the usual code paths so
nothing is missed.

Also, increment num_homeless_ops before we recalc_op_target(), so that we
don't (harmlessly, but confusingly) underflow.

Fixes: #2022
Signed-off-by: Sage Weil <sage@inktank.com>
---

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 3be08f22c6d7..15614c4510cc 100644
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -266,9 +266,9 @@ void Objecter::send_linger(LingerOp *info, bool first_send)
     }
 
     if (first_send) {
-      op_submit(o, info->session);
+      op_submit(o);
     } else {
-      _op_submit(o, info->session);
+      _op_submit(o);
     }
 
     OSDSession *s = o->session;
@@ -856,7 +856,7 @@ void Objecter::resend_mon_ops()
 
 // read | write ---------------------------
 
-tid_t Objecter::op_submit(Op *op, OSDSession *s)
+tid_t Objecter::op_submit(Op *op)
 {
   assert(client_lock.is_locked());
   assert(initialized);
@@ -869,10 +869,10 @@ tid_t Objecter::op_submit(Op *op, OSDSession *s)
   // take_op_budget() may drop our lock while it blocks.
   take_op_budget(op);
 
-  return _op_submit(op, s);
+  return _op_submit(op);
 }
 
-tid_t Objecter::_op_submit(Op *op, OSDSession *s)
+tid_t Objecter::_op_submit(Op *op)
 {
   // pick tid
   tid_t mytid = ++last_tid;
@@ -881,14 +881,9 @@ tid_t Objecter::_op_submit(Op *op, OSDSession *s)
 
   // pick target
   bool check_for_latest_map = false;
-  if (s) {
-    op->session = s;
-    s->ops.push_back(&op->session_item);
-  } else {
-    int r = recalc_op_target(op);
-    check_for_latest_map = (r == RECALC_OP_TARGET_POOL_DNE);
-    num_homeless_ops++;  // initially!
-  }
+  num_homeless_ops++;  // initially; recalc_op_target() will decrement if it finds a target
+  int r = recalc_op_target(op);
+  check_for_latest_map = (r == RECALC_OP_TARGET_POOL_DNE);
 
   // add to gather set(s)
   if (op->onack) {
diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index 61accb712fdc..2d405f1963b0 100644
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -961,8 +961,8 @@ public:
 
 private:
   // low-level
-  tid_t op_submit(Op *op, OSDSession *s = NULL);
-  tid_t _op_submit(Op *op, OSDSession *s);
+  tid_t op_submit(Op *op);
+  tid_t _op_submit(Op *op);
 
   // public interface
  public: