On 01.04.25 16:35, Eric Blake wrote:
On Tue, Mar 25, 2025 at 05:06:51PM +0100, Hanna Czenczek wrote:
Manually read requests from the /dev/fuse FD and process them, without
using libfuse.  This allows us to safely add parallel request processing
in coroutines later, without having to worry about libfuse internals.
(Technically, we already have exactly that problem with
read_from_fuse_export()/read_from_fuse_fd() nesting.)

We will continue to use libfuse for mounting the filesystem; fusermount3
is a effectively a helper program of libfuse, so it should know best how
to interact with it.  (Doing it manually without libfuse, while doable,
is a bit of a pain, and it is not clear to me how stable the "protocol"
actually is.)

@@ -247,6 +268,14 @@ static int fuse_export_create(BlockExport *blk_exp,
g_hash_table_insert(exports, g_strdup(exp->mountpoint), NULL); + exp->fuse_fd = fuse_session_fd(exp->fuse_session);
+    ret = fcntl(exp->fuse_fd, F_SETFL, O_NONBLOCK);
fctnl(F_SETFL) should be used in a read-modify-write pattern with
F_GETFL (otherwise, you are nuking any other flags that might have
been important).

See also block/file-posix.c:fcntl_setfl.  Maybe we should hoist that
into a common helper in util/osdep.c?

  /**
- * Handle client reads from the exported image.
+ * Handle client reads from the exported image.  Allocates *bufptr and reads
+ * data from the block device into that buffer.
Worth calling out tht *bufptr must be freed with qemu_vfree...

+ * Returns the buffer (read) size on success, and -errno on error.
   */
-static void fuse_read(fuse_req_t req, fuse_ino_t inode,
-                      size_t size, off_t offset, struct fuse_file_info *fi)
+static ssize_t fuse_read(FuseExport *exp, void **bufptr,
+                         uint64_t offset, uint32_t size)
...
  {
      buf = qemu_try_blockalign(blk_bs(exp->common.blk), size);
      if (!buf) {
-        fuse_reply_err(req, ENOMEM);
-        return;
+        return -ENOMEM;
      }
ret = blk_pread(exp->common.blk, offset, size, buf, 0);
-    if (ret >= 0) {
-        fuse_reply_buf(req, buf, size);
-    } else {
-        fuse_reply_err(req, -ret);
+    if (ret < 0) {
+        qemu_vfree(buf);
+        return ret;
...since internal cleanup recognizes that plain free() is wrong?

  #ifdef CONFIG_FUSE_LSEEK
  /**
   * Let clients inquire allocation status.
+ * Return the number of bytes written to *out on success, and -errno on error.
   */
-static void fuse_lseek(fuse_req_t req, fuse_ino_t inode, off_t offset,
-                       int whence, struct fuse_file_info *fi)
+static ssize_t fuse_lseek(FuseExport *exp, struct fuse_lseek_out *out,
+                          uint64_t offset, uint32_t whence)
  {
-    FuseExport *exp = fuse_req_userdata(req);
-
      if (whence != SEEK_HOLE && whence != SEEK_DATA) {
-        fuse_reply_err(req, EINVAL);
-        return;
+        return -EINVAL;
Unrelated to this patch, but any reason why we only SEEK_HOLE/DATA
(and not, say, SEEK_SET)?  Is it because we aren't really maintaining
the notion of a current offset?  I guess that works as long as the
caller is always using pread/pwrite (never bare read/write where
depending on our internal offset would matter).

Because FUSE doesn’t send SEEK_SET; FDs‘ in-file offsets are maintained by the kernel.

Hanna


Reply via email to