]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
client: fix error handling in check_pool_perm 4604/head
authorJohn Spray <john.spray@redhat.com>
Thu, 7 May 2015 17:42:01 +0000 (18:42 +0100)
committerJohn Spray <john.spray@redhat.com>
Thu, 7 May 2015 17:46:38 +0000 (18:46 +0100)
Previously, on an error such as a pool not existing,
the caller doing the check would error out, but
anyone waiting on waiting_for_pool_perm would
block indefinitely (symptom was that reads on a
file with a bogus layout would block forever).

Fix by triggering the wait list on errors and
clear the CHECKING state so that the other callers
also perform the check and find the error.

Additionally, don't return the RADOS error code
up to filesystem users, because it can be
misleading.  For example, nonexistent pool is
ENOENT, but we shouldn't give ENOENT on IO
to a file which does exist, we should give EIO.

Signed-off-by: John Spray <john.spray@redhat.com>
src/client/Client.cc

index 814ed358652b51dd47b8b212f9e5e5858d368dc5..063958f0c3b92f8dda9eb789d27a9a06e9452460 100644 (file)
@@ -10802,12 +10802,14 @@ int Client::check_pool_perm(Inode *in, int need)
     int wr_ret = wr_cond.wait();
     client_lock.Lock();
 
+    bool errored = false;
+
     if (rd_ret == 0 || rd_ret == -ENOENT)
       have |= POOL_READ;
     else if (rd_ret != -EPERM) {
       ldout(cct, 10) << "check_pool_perm on pool " << pool
                     << " rd_err = " << rd_ret << " wr_err = " << wr_ret << dendl;
-      return rd_ret;
+      errored = true;
     }
 
     if (wr_ret == 0 || wr_ret == -EEXIST)
@@ -10815,7 +10817,16 @@ int Client::check_pool_perm(Inode *in, int need)
     else if (wr_ret != -EPERM) {
       ldout(cct, 10) << "check_pool_perm on pool " << pool
                     << " rd_err = " << rd_ret << " wr_err = " << wr_ret << dendl;
-      return wr_ret;
+      errored = true;
+    }
+
+    if (errored) {
+      // Indeterminate: erase CHECKING state so that subsequent calls re-check.
+      // Raise EIO because actual error code might be misleading for
+      // userspace filesystem user.
+      pool_perms.erase(pool);
+      signal_cond_list(waiting_for_pool_perm);
+      return -EIO;
     }
 
     pool_perms[pool] = have | POOL_CHECKED;