From: Sage Weil <sage@newdream.net>
Date: Thu, 11 Nov 2010 04:58:49 +0000 (-0800)
Subject: mds: fix null_snapflush with multiple intervening snaps
X-Git-Tag: v0.23~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3d10b340748e5bbff86b49ac7386da9efa27a070;p=ceph-ci.git

mds: fix null_snapflush with multiple intervening snaps

The client is allowed to not send a snapflush if there is no dirty metadata
to write for a given snap.  However, the mds can only look up inodes by
the last snapid in the interval.  So, when doing a null_snapflush (filling
in for snapflushes the client didn't send), we have to walk forward through
intervening snaps until we find the right inode.

Note that this means we will call _do_snap_update multiple times on the
same inode, but with different snapids.

Add unit test to check this.

Signed-off-by: Sage Weil <sage@newdream.net>
---

diff --git a/qa/workunits/snaptest-double-null.sh b/qa/workunits/snaptest-double-null.sh
new file mode 100755
index 00000000000..cdf32e4f0ef
--- /dev/null
+++ b/qa/workunits/snaptest-double-null.sh
@@ -0,0 +1,23 @@
+#!/bin/sh -x
+
+set -e
+
+# multiple intervening snapshots with no modifications, and thus no
+# snapflush client_caps messages.  make sure the mds can handle this.
+
+for f in `seq 1 20` ; do
+
+mkdir a
+cat > a/foo &
+mkdir a/.snap/one
+mkdir a/.snap/two
+chmod 777 a/foo
+sync   # this might crash the mds
+ps
+rmdir a/.snap/*
+rm a/foo
+rmdir a
+
+done
+
+echo OK
diff --git a/src/mds/Locker.cc b/src/mds/Locker.cc
index 27a26f19b1c..1cddcf83f92 100644
--- a/src/mds/Locker.cc
+++ b/src/mds/Locker.cc
@@ -1805,7 +1805,7 @@ void Locker::_do_null_snapflush(CInode *head_in, client_t client, snapid_t follo
   while (p != head_in->client_need_snapflush.end()) {
     snapid_t snapid = p->first;
     set<client_t>& clients = p->second;
-    p++;
+    p++;  // be careful, q loop below depends on this
 
     // snapid is the snap inode's ->last
     if (follows > snapid)
@@ -1813,9 +1813,25 @@ void Locker::_do_null_snapflush(CInode *head_in, client_t client, snapid_t follo
     if (clients.count(client)) {
       dout(10) << " doing async NULL snapflush on " << snapid << " from client" << client << dendl;
       CInode *sin = mdcache->get_inode(head_in->ino(), snapid);
-      if (!sin && head_in->is_multiversion())
-	sin = head_in;
-      assert(sin);
+      if (!sin) {
+	// hrm, look forward until we find the inode. 
+	//  (we can only look it up by the last snapid it is valid for)
+	dout(10) << " didn't have " << head_in->ino() << " snapid " << snapid << dendl;
+	for (map<snapid_t, set<client_t> >::iterator q = p;  // p is already at next entry
+	     q != head_in->client_need_snapflush.end();
+	     q++) {
+	  dout(10) << " trying snapid " << q->first << dendl;
+	  sin = mdcache->get_inode(head_in->ino(), q->first);
+	  if (sin) {
+	    assert(sin->first <= snapid);
+	    break;
+	  }
+	  dout(10) << " didn't have " << head_in->ino() << " snapid " << q->first << dendl;
+	}
+	if (!sin && head_in->is_multiversion())
+	  sin = head_in;
+	assert(sin);
+      }
       _do_snap_update(sin, snapid, 0, sin->first - 1, client, NULL, NULL);
       head_in->remove_need_snapflush(sin, snapid, client);
     }