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 (executable)
index 0000000..cdf32e4
--- /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 27a26f19b1cb6c48f5cb06467fabc30ee237344c..1cddcf83f926724b7169e2e29a314adb145863e4 100644 (file)
--- 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);
     }