client: cleanup ::_read return

::_read should be returning the number of bytes read, not the length of the
buffer. If the buffer already had data in it, the return value is wrong.

Fixes: https://tracker.ceph.com/issues/23293
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>

diff --git a/src/client/Client.cc b/src/client/Client.cc
index bb0163b11efa96bb7da01b59ff410e0e55d82fc6..f40570cddd404eb79455e1e28782984d03ee4280 100644 (file)
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -8742,6 +8742,10 @@ int Client::preadv(int fd, const struct iovec *iov, int iovcnt, loff_t offset)
 
 int64_t Client::_read(Fh *f, int64_t offset, uint64_t size, bufferlist *bl)
 {
+  int have = 0;
+  bool movepos = false;
+  std::unique_ptr<C_SaferCond> onuninline;
+  int64_t r;
   const md_config_t *conf = cct->_conf;
   Inode *in = f->inode.get();
 
@@ -8749,7 +8753,6 @@ int64_t Client::_read(Fh *f, int64_t offset, uint64_t size, bufferlist *bl)
     return -EBADF;
   //bool lazy = f->mode == CEPH_FILE_MODE_LAZY;
 
-  bool movepos = false;
   if (offset < 0) {
     lock_fh_pos(f);
     offset = f->pos;
@@ -8760,26 +8763,20 @@ int64_t Client::_read(Fh *f, int64_t offset, uint64_t size, bufferlist *bl)
   if (in->inline_version == 0) {
     int r = _getattr(in, CEPH_STAT_CAP_INLINE_DATA, f->actor_perms, true);
     if (r < 0) {
-      if (movepos)
-        unlock_fh_pos(f);
-      return r;
+      goto done;
     }
     assert(in->inline_version > 0);
   }
 
 retry:
-  int have;
-  int r = get_caps(in, CEPH_CAP_FILE_RD, CEPH_CAP_FILE_CACHE, &have, -1);
+  have = 0;
+  r = get_caps(in, CEPH_CAP_FILE_RD, CEPH_CAP_FILE_CACHE, &have, -1);
   if (r < 0) {
-    if (movepos)
-        unlock_fh_pos(f);
-    return r;
+    goto done;
   }
   if (f->flags & O_DIRECT)
     have &= ~CEPH_CAP_FILE_CACHE;
 
-  std::unique_ptr<C_SaferCond> onuninline = nullptr;
-
   if (in->inline_version < CEPH_INLINE_NONE) {
     if (!(have & CEPH_CAP_FILE_CACHE)) {
       onuninline.reset(new C_SaferCond("Client::_read_uninline_data flock"));
@@ -8841,16 +8838,16 @@ retry:
   }
 
 success:
+  assert(r >= 0);
   if (movepos) {
     // adjust fd pos
-    f->pos = start_pos + bl->length();
-    unlock_fh_pos(f);
+    f->pos = start_pos + r;
   }
 
 done:
   // done!
 
-  if (nullptr != onuninline) {
+  if (onuninline) {
     client_lock.Unlock();
     int ret = onuninline->wait();
     client_lock.Lock();
@@ -8862,15 +8859,13 @@ done:
     } else
       r = ret;
   }
-
-  if (have)
+  if (have) {
     put_cap_ref(in, CEPH_CAP_FILE_RD);
-  if (r < 0) {
-    if (movepos)
-        unlock_fh_pos(f);
-    return r;
-  } else
-    return bl->length();
+  }
+  if (movepos) {
+    unlock_fh_pos(f);
+  }
+  return r;
 }
 
 Client::C_Readahead::C_Readahead(Client *c, Fh *f) :