From: Kevin Wolf <kwolf@redhat.com>
To: Brian Song <hibriansong@gmail.com>
Cc: qemu-block@nongnu.org, qemu-devel@nongnu.org, armbru@redhat.com,
bschubert@ddn.com, fam@euphon.net, hreitz@redhat.com,
stefanha@redhat.com
Subject: Re: [PATCH RFC 1/1] block/export: FUSE-over-io_uring Support for QEMU FUSE Exports
Date: Tue, 22 Jul 2025 15:32:47 +0200 [thread overview]
Message-ID: <aH-S_31c8RNW3coY@redhat.com> (raw)
In-Reply-To: <20250716183824.216257-2-hibriansong@gmail.com>
Am 16.07.2025 um 20:38 hat Brian Song geschrieben:
> This work provides an initial implementation of fuse-over-io_uring
> support for QEMU export. According to the fuse-over-io_uring protocol
> specification, the userspace side must create the same number of queues
> as the number of CPUs (nr_cpu), just like the kernel. Currently, each
> queue contains only a single SQE entry, which is used to validate the
> correctness of the fuse-over-io_uring functionality.
>
> All FUSE read and write operations interact with the kernel via io
> vectors embedded in the SQE entry during submission and CQE fetching.
> The req_header and op_payload members of each entry are included as
> parts of the io vector: req_header carries the FUSE operation header,
> and op_payload carries the data payload, such as file attributes in a
> getattr reply, file content in a read reply, or file content being
> written to the FUSE client in a write operation.
>
> At present, multi-threading support is still incomplete. In addition,
> handling connection termination and managing the "drained" state of a
> FUSE block export in QEMU remain as pending work.
>
> Suggested-by: Kevin Wolf <kwolf@redhat.com>
> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Brian Song <hibriansong@gmail.com>
>
> ---
> block/export/fuse.c | 423 +++++++++++++++++++++++++--
> docs/tools/qemu-storage-daemon.rst | 10 +-
> qapi/block-export.json | 6 +-
> storage-daemon/qemu-storage-daemon.c | 1 +
> util/fdmon-io_uring.c | 5 +-
> 5 files changed, 420 insertions(+), 25 deletions(-)
You already got a lot of feedback on details. Let me add a more generic
point that should be addressed for a non-RFC submission: You should try
to limit each commit to a single logical change. You'll then send a
patch series instead of just a single patch where each patch works
incrementally towards the final state.
This makes it not only easier to review the changes because there is
less going on in each individual patch, but it will also be helpful if
we ever have to debug a problem and can bisect to a smaller change, or
even just because looking at the commit message in a few years is
likelier to explain why some specific code was written the way it is.
For example, your change to util/fdmon-io_uring.c could be the first
patch, enabling IORING_SETUP_SQE128 is a self-contained logical change.
You can mention in its commit message that it's in preparation for using
IORING_OP_URING_CMD.
Another patch could be refactoring fuse_co_process_request() so that it
works for both /dev/fuse based and io_uring based exports. (I agree with
Stefan that the code should be shared between both.)
And then you could see if adding io_uring support to the FUSE export
itself can be broken down into multiple self-contained changes or if
that part has to stay a single big patch. Maybe you can keep it similar
to how you're actually developing the code: First a patch with a minimal
implementation that processes one request per queue, then another one to
implement parallel I/O, then one for supporting multiple iothreads.
Another thing I wondered is if the large #ifdef'ed section with io_uring
helpers would actually make more sense as a separate source file
block/export/fuse-io_uring.c that can be linked conditionally if
io_uring is available. This would help to minimise the number of #ifdefs
in fuse.c.
Kevin
next prev parent reply other threads:[~2025-07-22 13:34 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-07-16 18:38 [PATCH RFC 0/1] block/export: FUSE-over-io_uring Support for QEMU FUSE Exports Brian Song
2025-07-16 18:38 ` [PATCH RFC 1/1] " Brian Song
2025-07-17 6:03 ` Markus Armbruster
2025-07-22 12:00 ` Brian Song
2025-07-21 0:53 ` Stefan Hajnoczi
2025-07-22 12:00 ` Brian Song
2025-07-22 15:17 ` Kevin Wolf
2025-07-22 14:06 ` Bernd Schubert
2025-07-22 15:43 ` Stefan Hajnoczi
2025-07-22 16:20 ` Bernd Schubert
2025-07-21 13:51 ` Bernd Schubert
2025-07-21 18:26 ` Stefan Hajnoczi
2025-07-22 12:00 ` Brian Song
2025-07-22 14:51 ` Stefan Hajnoczi
2025-07-24 20:36 ` Stefan Hajnoczi
2025-07-22 13:32 ` Kevin Wolf [this message]
2025-12-10 22:10 ` Bernd Schubert
2025-12-11 4:40 ` Brian Song
2025-07-20 16:13 ` [PATCH RFC 0/1] " Stefan Hajnoczi
2025-07-22 12:00 ` Brian Song
2025-07-22 14:47 ` Stefan Hajnoczi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aH-S_31c8RNW3coY@redhat.com \
--to=kwolf@redhat.com \
--cc=armbru@redhat.com \
--cc=bschubert@ddn.com \
--cc=fam@euphon.net \
--cc=hibriansong@gmail.com \
--cc=hreitz@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.