cleanup and pass the tests

qa/tasks/cephfs/test_quiesce.py

@@ -205,20 +205,13 @@ def _verify_quiesce(self, rank=0, root=None, splitauth=False):
                     lock_type = lock['lock']['type']
                     if lock_type == "iquiesce":
                         self.assertEqual(lock['flags'], 4)
-                        self.assertEqual(lock['lock']['state'], 'xlock')
-                    elif lock_type == "isnap":
-                        self.assertEqual(lock['flags'], 1)
-                        self.assertEqual(lock['lock']['state'][:4], 'sync')
-                    elif lock_type == "ifile":
-                        if (mode & S_IFMT) == S_IFREG:
-                            self.assertEqual(lock['flags'], 4)
-                            self.assertEqual(lock['lock']['state'], 'xlock')
-                        else:
-                            self.assertEqual(lock['flags'], 1)
-                            self.assertEqual(lock['lock']['state'][:4], 'sync')
-                    elif lock_type in ("iauth", "ilink", "ixattr"):
-                        self.assertEqual(lock['flags'], 1)
-                        self.assertEqual(lock['lock']['state'][:4], 'sync')
+                        self.assertEqual(lock['lock']['state'], 'lock')
+                        self.assertEqual(lock['lock']['num_xlocks'], 1)
+                    elif lock_type == "iversion":
+                        # only regular files acquire xlock (on ifile)
+                        self.assertEqual((mode & S_IFMT), S_IFREG)
+                        self.assertEqual(lock['flags'], 2)
+                        self.assertEqual(lock['lock']['state'][:4], 'lock')
                     else:
                         # no iflock
                         self.assertFalse(lock_type.startswith("i"))

src/mds/Locker.cc

@@ -606,11 +606,7 @@ bool Locker::acquire_locks(const MDRequestRef& mdr,
       if (mdr->locking && lock != mdr->locking)
 	cancel_locking(mdr.get(), &issue_set);
       if (!xlock_start(lock, mdr)) {
-        if (t == CEPH_LOCK_IQUIESCE) {
-          marker.message = "failed to xlock quiesce, waiting";
-        } else {
-	  marker.message = "failed to xlock, waiting";
-        }
+	marker.message = "failed to xlock, waiting";
 	goto out;
       }
       dout(10) << " got xlock on " << *lock << " " << *lock->get_parent() << dendl;
@@ -686,11 +682,8 @@ bool Locker::acquire_locks(const MDRequestRef& mdr,
       }
 
       if (!rdlock_start(lock, mdr)) {
-        if (t == CEPH_LOCK_IQUIESCE) {
-          handle_quiesce_failure(mdr, marker.message);
-        } else {
-	  marker.message = "failed to rdlock, waiting";
-        }
+        ceph_assert(t != CEPH_LOCK_IQUIESCE); // rdlock is undefined for LocalLock
+        marker.message = "failed to rdlock, waiting";
 	goto out;
       }
       dout(10) << " got rdlock on " << *lock << " " << *lock->get_parent() << dendl;
@@ -1486,8 +1479,7 @@ bool Locker::eval(CInode *in, int mask, bool caps_imported)
     eval_any(&in->flocklock, &need_issue, &finishers, caps_imported);
   if (mask & CEPH_LOCK_IPOLICY)
     eval_any(&in->policylock, &need_issue, &finishers, caps_imported);
-  if (mask & CEPH_LOCK_IQUIESCE)
-    eval_any(&in->quiescelock, &need_issue, &finishers, caps_imported);
+  // LocalLocks should not be eval'd
 
   // drop loner?
   if (in->is_auth() && in->is_head() && in->get_wanted_loner() != in->get_loner()) {

src/mds/MDCache.cc

@@ -13547,7 +13547,7 @@ void MDCache::dispatch_quiesce_inode(const MDRequestRef& mdr)
    * The solution is probably to have rank1 mark /foo has STATE_QUIESCED and reject export ops from rank0.
    */
 
-  if (in->is_auth() || splitauth) {
+  if (in->is_auth()) {
     /* Acquire all caps-related locks to revoke unwanted caps
      *
      * The xlock on the quiescelock is important to prevent future requests
@@ -13558,36 +13558,48 @@ void MDCache::dispatch_quiesce_inode(const MDRequestRef& mdr)
     // take the quiesce lock and the first of the four capability releated locks
     lov.add_xlock(&in->quiescelock); /* !! */
 
-    /*
-     * Because files are treated specially allowing multiple reader/writers, we
-     * need an xlock here to recall all write caps. This unfortunately means
-     * there can be no readers.
-     */
-    lov.add_xlock(&in->filelock);
-    lov.add_xlock(&in->authlock);
-    lov.add_xlock(&in->linklock);
-    lov.add_xlock(&in->xattrlock);
+    lov.add_rdlock(&in->xattrlock);
+
+    if (!mds->locker->acquire_locks(mdr, lov, nullptr, { in }, false, true)) {
+      return;
+    }
+
+    // now we can test if this inode should be skipped
+    if (in->get_projected_inode()->get_quiesce_block()) {
+      dout(10) << __func__ << " quiesce is blocked for this inode; dropping locks!" << dendl;
+      mdr->mark_event("quiesce blocked");
+      mds->locker->drop_locks(mdr.get());
+      /* keep authpins! */
+      qs.inc_inodes_blocked();
+      mdr->internal_op_finish->complete(0);
+      mdr->internal_op_finish = nullptr;
+      return;
+    }
+
+    if (!in->is_dir()) {
+      /*
+       * Because files are treated specially allowing multiple reader/writers, we
+       * need an xlock here to recall all write caps. This unfortunately means
+       * there can be no readers.
+       */
+      lov.add_xlock(&in->filelock);
+    } else {
+      lov.add_rdlock(&in->filelock);
+    }
+
+    lov.add_rdlock(&in->authlock);
+    lov.add_rdlock(&in->linklock);
 
     if (!mds->locker->acquire_locks(mdr, lov, nullptr, {in}, false, true)) {
       return;
     }
-  } else {
+  } else if (!splitauth) {
     dout(5) << "auth is split and splitauth is false: " << *in << dendl;
     qs.add_failed(mdr, -EPERM);
     mds->server->respond_to_request(mdr, -EPERM);
     return;
   }
 
-  if (in->get_projected_inode()->get_quiesce_block()) {
-    dout(10) << __func__ << " quiesce is blocked for this inode; dropping locks!" << dendl;
-    mdr->mark_event("quiesce blocked");
-    mds->locker->drop_locks(mdr.get());
-    /* keep authpins! */
-    qs.inc_inodes_blocked();
-    mdr->internal_op_finish->complete(0);
-    mdr->internal_op_finish = nullptr;
-    return;
-  }
 
   // since we're holding onto the quiesce lock,
   // we don't need the other capabilities related locks anymore