Merge PR ceph#54690 into main

* refs/pull/54690/head:
	client: handle callback completion if the async I/O failed
	client: make sure the callback is finished when returning ENOTCONN
	client: do not accept zero byte write request
	client: check for negative value of iovcnt
	src/test: test zero bytes async i/o
	src/test: test async I/O with negative iov structures count
	src/test: test async I/O if the client is not mounted
	src/test: test async I/O with read only file
	src/test: test async I/O with a file created with O_PATH

Reviewed-by: Frank S. Filz <[email protected]>
Reviewed-by: Milind Changire <[email protected]>
Reviewed-by: Venky Shankar <[email protected]>
Reviewed-by: Rishabh Dave <[email protected]>
Reviewed-by: Matt Benjamin <[email protected]>

Author: vshankar

src/client/Client.cc

@@ -10655,8 +10655,6 @@ int Client::read(int fd, char *buf, loff_t size, loff_t offset)
 
 int Client::preadv(int fd, const struct iovec *iov, int iovcnt, loff_t offset)
 {
-  if (iovcnt < 0)
-    return -CEPHFS_EINVAL;
   return _preadv_pwritev(fd, iov, iovcnt, offset, false);
 }
 
@@ -11219,13 +11217,11 @@ int Client::write(int fd, const char *buf, loff_t size, loff_t offset)
 
 int Client::pwritev(int fd, const struct iovec *iov, int iovcnt, int64_t offset)
 {
-  if (iovcnt < 0)
-    return -CEPHFS_EINVAL;
   return _preadv_pwritev(fd, iov, iovcnt, offset, true);
 }
 
 int64_t Client::_preadv_pwritev_locked(Fh *fh, const struct iovec *iov,
-                                       unsigned iovcnt, int64_t offset,
+                                       int iovcnt, int64_t offset,
                                        bool write, bool clamp_to_int,
                                        Context *onfinish, bufferlist *blp,
                                        bool do_fsync, bool syncdataonly)
@@ -11236,8 +11232,11 @@ int64_t Client::_preadv_pwritev_locked(Fh *fh, const struct iovec *iov,
     if (fh->flags & O_PATH)
         return -CEPHFS_EBADF;
 #endif
+    if(iovcnt < 0) {
+      return -CEPHFS_EINVAL;
+    }
     loff_t totallen = 0;
-    for (unsigned i = 0; i < iovcnt; i++) {
+    for (int i = 0; i < iovcnt; i++) {
         totallen += iov[i].iov_len;
     }
 
@@ -11260,11 +11259,6 @@ int64_t Client::_preadv_pwritev_locked(Fh *fh, const struct iovec *iov,
                           onfinish);
         ldout(cct, 3) << "preadv(" << fh << ", " <<  offset << ") = " << r << dendl;
         if (r <= 0) {
-          if (r < 0 && onfinish != nullptr) {
-            client_lock.unlock();
-            onfinish->complete(r);
-            client_lock.lock();
-          }
           return r;
         }
 
@@ -11275,7 +11269,7 @@ int64_t Client::_preadv_pwritev_locked(Fh *fh, const struct iovec *iov,
     }
 }
 
-int Client::_preadv_pwritev(int fd, const struct iovec *iov, unsigned iovcnt,
+int Client::_preadv_pwritev(int fd, const struct iovec *iov, int iovcnt,
                             int64_t offset, bool write, Context *onfinish,
                             bufferlist *blp)
 {
@@ -11442,6 +11436,10 @@ int64_t Client::_write(Fh *f, int64_t offset, uint64_t size, const char *buf,
   CWF_iofinish *cwf_iofinish = NULL;
   C_SaferCond *cond_iofinish = NULL;
 
+  if (size < 1) { // zero bytes write is not supported by osd
+    return -CEPHFS_EINVAL;
+  }
+
   if ( (uint64_t)(offset+size) > mdsmap->get_max_filesize() && //exceeds config
        (uint64_t)(offset+size) > in->size ) { //exceeds filesize 
       return -CEPHFS_EFBIG;              
@@ -15945,12 +15943,49 @@ int64_t Client::ll_preadv_pwritev(struct Fh *fh, const struct iovec *iov,
                                   bool do_fsync, bool syncdataonly)
 {
     RWRef_t mref_reader(mount_state, CLIENT_MOUNTING);
-    if (!mref_reader.is_state_satisfied())
-      return -CEPHFS_ENOTCONN;
+    if (!mref_reader.is_state_satisfied()) {
+      int64_t rc = -CEPHFS_ENOTCONN;
+      if (onfinish != nullptr) {
+        onfinish->complete(rc);
+        /* async call should always return zero to caller and allow the
+        caller to wait on callback for the actual errno. */
+        rc = 0;
+      }
+      return rc;
+    }
 
     std::scoped_lock cl(client_lock);
-    return _preadv_pwritev_locked(fh, iov, iovcnt, offset, write, true,
-    				  onfinish, bl, do_fsync, syncdataonly);
+
+    int64_t retval = _preadv_pwritev_locked(fh, iov, iovcnt, offset, write,
+                                            true, onfinish, bl, do_fsync,
+                                            syncdataonly);
+    /* There are two scenarios with each having two cases to handle here
+    1) async io
+      1.a) r == 0:
+        async call in progress, the context will be automatically invoked,
+        so just return the retval (i.e. zero).
+      1.b) r < 0:
+        There was an error; no context completion should've took place so
+        complete the context with retval followed by returning zero to the
+        caller.
+    2) sync io
+      2.a) r >= 0:
+        sync call success; return the no. of bytes read/written.
+      2.b) r < 0:
+        sync call failed; return the errno. */
+
+    if (retval < 0) {
+      if (onfinish != nullptr) {
+        //async io failed
+        client_lock.unlock();
+        onfinish->complete(retval);
+        client_lock.lock();
+        /* async call should always return zero to caller and allow the
+        caller to wait on callback for the actual errno/retval. */
+        retval = 0;
+      }
+    }
+    return retval;
 }
 
 int Client::ll_flush(Fh *fh)

src/client/Client.h

@@ -1677,12 +1677,12 @@ class Client : public Dispatcher, public md_config_obs_t {
           const struct iovec *iov, int iovcnt, Context *onfinish = nullptr,
           bool do_fsync = false, bool syncdataonly = false);
   int64_t _preadv_pwritev_locked(Fh *fh, const struct iovec *iov,
-                                 unsigned iovcnt, int64_t offset,
+                                 int iovcnt, int64_t offset,
                                  bool write, bool clamp_to_int,
                                  Context *onfinish = nullptr,
                                  bufferlist *blp = nullptr,
                                  bool do_fsync = false, bool syncdataonly = false);
-  int _preadv_pwritev(int fd, const struct iovec *iov, unsigned iovcnt,
+  int _preadv_pwritev(int fd, const struct iovec *iov, int iovcnt,
                       int64_t offset, bool write, Context *onfinish = nullptr,
                       bufferlist *blp = nullptr);
   int _flush(Fh *fh);