unix: update uv_fs_copyfile() fallback logic

This commit fixes uv_fs_copyfile() in cases where an unknown error
occurs when copy-on-write is requested by setting
UV_FS_COPYFILE_FICLONE. The original approach tried to catch some of
the errors raised by the ioctl() call, assuming that sendfile() would
also fail in those cases. This is not necessarily true, as some
variants of ioctl() also raise EINVAL (some maybe EBADF), but sendfile()
works just fine.

This patch reverses the logic, falling back to sendfile() in any
case where ioctl() returns an error. In other words, it tries much
harder to make uv_fs_copyfile() work.

Related to that, the original approach returned UV_ENOTSUP
unconditionally in cases where ioctl() failed and
UV_FS_COPYFILE_FICLONE_FORCE was set. However, ioctl() may have
failed for other reasons than being not supported. The function
now returns the actual error raised by ioctl(), leaving it to the
caller to deal with it.

Fixes: https://github.com/libuv/libuv/issues/2483
PR-URL: https://github.com/libuv/libuv/pull/2514
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

docs/src/fs.rst

@@ -359,10 +359,13 @@ API
       is to overwrite the destination if it exists.
     - `UV_FS_COPYFILE_FICLONE`: If present, `uv_fs_copyfile()` will attempt to
       create a copy-on-write reflink. If the underlying platform does not
-      support copy-on-write, then a fallback copy mechanism is used.
+      support copy-on-write, or an error occurs while attempting to use
+      copy-on-write, a fallback copy mechanism based on
+      :c:func:`uv_fs_sendfile()` is used.
     - `UV_FS_COPYFILE_FICLONE_FORCE`: If present, `uv_fs_copyfile()` will
       attempt to create a copy-on-write reflink. If the underlying platform does
-      not support copy-on-write, then an error is returned.
+      not support copy-on-write, or an error occurs while attempting to use
+      copy-on-write, then an error is returned.
 
     .. warning::
         If the destination path is created, but an error occurs while copying
@@ -375,6 +378,10 @@ API
     .. versionchanged:: 1.20.0 `UV_FS_COPYFILE_FICLONE` and
         `UV_FS_COPYFILE_FICLONE_FORCE` are supported.
 
+    .. versionchanged:: 1.33.0 If an error occurs while using
+        `UV_FS_COPYFILE_FICLONE_FORCE`, that error is returned. Previously,
+        all errors were mapped to `UV_ENOTSUP`.
+
 .. c:function:: int uv_fs_sendfile(uv_loop_t* loop, uv_fs_t* req, uv_file out_fd, uv_file in_fd, int64_t in_offset, size_t length, uv_fs_cb cb)
 
     Limited equivalent to :man:`sendfile(2)`.

src/unix/fs.c

@@ -1055,18 +1055,14 @@ static ssize_t uv__fs_copyfile(uv_fs_t* req) {
 #ifdef FICLONE
   if (req->flags & UV_FS_COPYFILE_FICLONE ||
       req->flags & UV_FS_COPYFILE_FICLONE_FORCE) {
-    if (ioctl(dstfd, FICLONE, srcfd) == -1) {
-      /* If an error occurred that the sendfile fallback also won't handle, or
-         this is a force clone then exit. Otherwise, fall through to try using
-         sendfile(). */
-      if (errno != ENOTTY && errno != EOPNOTSUPP && errno != EXDEV) {
-        err = UV__ERR(errno);
-        goto out;
-      } else if (req->flags & UV_FS_COPYFILE_FICLONE_FORCE) {
-        err = UV_ENOTSUP;
-        goto out;
-      }
-    } else {
+    if (ioctl(dstfd, FICLONE, srcfd) == 0) {
+      /* ioctl() with FICLONE succeeded. */
+      goto out;
+    }
+    /* If an error occurred and force was set, return the error to the caller;
+     * fall back to sendfile() when force was not set. */
+    if (req->flags & UV_FS_COPYFILE_FICLONE_FORCE) {
+      err = UV__ERR(errno);
       goto out;
     }
   }

test/test-fs-copyfile.c

@@ -188,7 +188,7 @@ TEST_IMPL(fs_copyfile) {
   unlink(dst);
   r = uv_fs_copyfile(NULL, &req, fixture, dst, UV_FS_COPYFILE_FICLONE_FORCE,
                      NULL);
-  ASSERT(r == 0 || r == UV_ENOSYS || r == UV_ENOTSUP);
+  ASSERT(r <= 0);
 
   if (r == 0)
     handle_result(&req);