client: hold reference for newly updated snaprealm

Client::update_snap_trace() may create new snaprealm, then update
them. When Client::update_snap_trace() return, the newly created
snaprealm get freed immediately. This is wrong because callers of
Client::update_snap_trace() expects Client::get_snap_realm() return
the updated snaprealm.

Signed-off-by: "Yan, Zheng" <zyan@redhat.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 68b15e685475373f6bef155c2ce5561b163f0aa1..c52d97a25d15d7904325e41d4353283ec93b8fbe 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -1267,8 +1267,9 @@ Inode* Client::insert_trace(MetaRequest *request, MetaSession *session)
   ldout(cct, 10) << " features 0x" << hex << features << dec << dendl;
 
   // snap trace
+  SnapRealm *realm = NULL;
   if (reply->snapbl.length())
-    update_snap_trace(reply->snapbl);
+    update_snap_trace(reply->snapbl, &realm);
 
   ldout(cct, 10) << " hrm " 
           << " is_target=" << (int)reply->head.is_target
@@ -1376,6 +1377,9 @@ Inode* Client::insert_trace(MetaRequest *request, MetaSession *session)
     }
   }
 
+  if (realm)
+    put_snap_realm(realm);
+
   request->target = in;
   return in;
 }
@@ -4345,9 +4349,9 @@ static bool has_new_snaps(const SnapContext& old_snapc,
 }
 
 
-inodeno_t Client::update_snap_trace(bufferlist& bl, bool flush)
+void Client::update_snap_trace(bufferlist& bl, SnapRealm **realm_ret, bool flush)
 {
-  inodeno_t first_realm = 0;
+  SnapRealm *first_realm = NULL;
   ldout(cct, 10) << "update_snap_trace len " << bl.length() << dendl;
 
   map<SnapRealm*, SnapContext> dirty_realms;
@@ -4356,8 +4360,6 @@ inodeno_t Client::update_snap_trace(bufferlist& bl, bool flush)
   while (!p.end()) {
     SnapRealmInfo info;
     ::decode(info, p);
-    if (first_realm == 0)
-      first_realm = info.ino();
     SnapRealm *realm = get_snap_realm(info.ino());
 
     bool invalidate = false;
@@ -4409,7 +4411,10 @@ inodeno_t Client::update_snap_trace(bufferlist& bl, bool flush)
               << " <= " << realm->seq << " and same parent, SKIPPING" << dendl;
     }
         
-    put_snap_realm(realm);
+    if (!first_realm)
+      first_realm = realm;
+    else
+      put_snap_realm(realm);
   }
 
   for (map<SnapRealm*, SnapContext>::iterator q = dirty_realms.begin();
@@ -4431,7 +4436,10 @@ inodeno_t Client::update_snap_trace(bufferlist& bl, bool flush)
     put_snap_realm(realm);
   }
 
-  return first_realm;
+  if (realm_ret)
+    *realm_ret = first_realm;
+  else
+    put_snap_realm(first_realm);
 }
 
 void Client::handle_snap(MClientSnap *m)
@@ -4494,7 +4502,7 @@ void Client::handle_snap(MClientSnap *m)
     }
   }
 
-  update_snap_trace(m->bl, m->head.op != CEPH_SNAP_OP_DESTROY);
+  update_snap_trace(m->bl, NULL, m->head.op != CEPH_SNAP_OP_DESTROY);
 
   if (realm) {
     for (auto p = to_move.begin(); p != to_move.end(); ++p) {
@@ -4631,7 +4639,9 @@ void Client::handle_cap_import(MetaSession *session, Inode *in, MClientCaps *m)
   }
 
   // add/update it
-  update_snap_trace(m->snapbl);
+  SnapRealm *realm = NULL;
+  update_snap_trace(m->snapbl, &realm);
+
   add_update_cap(in, session, m->get_cap_id(),
                 m->get_caps(), m->get_seq(), m->get_mseq(), m->get_realm(),
                 CEPH_CAP_FLAG_AUTH, cap_perms);
@@ -4639,6 +4649,9 @@ void Client::handle_cap_import(MetaSession *session, Inode *in, MClientCaps *m)
   if (cap && cap->cap_id == m->peer.cap_id) {
       remove_cap(cap, (m->peer.flags & CEPH_CAP_FLAG_RELEASE));
   }
+
+  if (realm)
+    put_snap_realm(realm);
   
   if (in->auth_cap && in->auth_cap->session->mds_num == mds) {
     // reflush any/all caps (if we are now the auth_cap)

diff --git a/src/client/Client.h b/src/client/Client.h
index baa79ab94810671554627095ef3cfbe8a94288f8..e9fa90416e4f9c74f4880775aa0633d4c283ca6e 100644 (file)
--- a/src/client/Client.h
+++ b/src/client/Client.h
@@ -443,8 +443,7 @@ protected:
   SnapRealm *get_snap_realm_maybe(inodeno_t r);
   void put_snap_realm(SnapRealm *realm);
   bool adjust_realm_parent(SnapRealm *realm, inodeno_t parent);
-  inodeno_t update_snap_trace(bufferlist& bl, bool must_flush=true);
-  inodeno_t _update_snap_trace(vector<SnapRealmInfo>& trace);
+  void update_snap_trace(bufferlist& bl, SnapRealm **realm_ret, bool must_flush=true);
   void invalidate_snaprealm_and_children(SnapRealm *realm);
 
   Inode *open_snapdir(Inode *diri);