On Thu, Sep 11, 2025 at 10:22:24PM -0400, Brian Song wrote:
>
>
> On 9/10/25 9:14 AM, Stefan Hajnoczi wrote:
> > On Tue, Sep 09, 2025 at 04:51:12PM -0400, Brian Song wrote:
> > >
> > >
> > > On 9/9/25 3:38 PM, Stefan Hajnoczi wrote:
> > > > On Fri, Aug 29, 2025 at 10:50:25PM -0400, Brian Song wrote:
> > > > > To test FUSE-over-io_uring, set the environment variable
> > > > > FUSE_OVER_IO_URING=1. This applies only when using the
> > > > > 'fuse' protocol.
> > > > >
> > > > > $ FUSE_OVER_IO_URING=1 ./check -fuse
> > > > >
> > > > > Suggested-by: Kevin Wolf <kw...@redhat.com>
> > > > > Suggested-by: Stefan Hajnoczi <stefa...@redhat.com>
> > > > > Signed-off-by: Brian Song <hibrians...@gmail.com>
> > > > > ---
> > > > > tests/qemu-iotests/check | 2 ++
> > > > > tests/qemu-iotests/common.rc | 45
> > > > > +++++++++++++++++++++++++++---------
> > > > > 2 files changed, 36 insertions(+), 11 deletions(-)
> > > > >
> > > > > diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> > > > > index 545f9ec7bd..c6fa0f9e3d 100755
> > > > > --- a/tests/qemu-iotests/check
> > > > > +++ b/tests/qemu-iotests/check
> > > > > @@ -94,6 +94,8 @@ def make_argparser() -> argparse.ArgumentParser:
> > > > > mg.add_argument('-' + fmt, dest='imgfmt',
> > > > > action='store_const',
> > > > > const=fmt, help=f'test {fmt}')
> > > > > + # To test FUSE-over-io_uring, set the environment variable
> > > > > + # FUSE_OVER_IO_URING=1. This applies only when using the 'fuse'
> > > > > protocol
> > > > > protocol_list = ['file', 'rbd', 'nbd', 'ssh', 'nfs', 'fuse']
> > > > > g_prt = p.add_argument_group(
> > > > > ' image protocol options',
> > > > > diff --git a/tests/qemu-iotests/common.rc
> > > > > b/tests/qemu-iotests/common.rc
> > > > > index e977cb4eb6..f8b79c3810 100644
> > > > > --- a/tests/qemu-iotests/common.rc
> > > > > +++ b/tests/qemu-iotests/common.rc
> > > > > @@ -539,17 +539,38 @@ _make_test_img()
> > > > > touch "$export_mp"
> > > > > rm -f "$SOCK_DIR/fuse-output"
> > > > > - # Usually, users would export formatted nodes. But we
> > > > > present fuse as a
> > > > > - # protocol-level driver here, so we have to leave the format
> > > > > to the
> > > > > - # client.
> > > > > - # Switch off allow-other, because in general we do not need
> > > > > it for
> > > > > - # iotests. The default allow-other=auto has the downside of
> > > > > printing a
> > > > > - # fusermount error on its first attempt if allow_other is not
> > > > > - # permissible, which we would need to filter.
> > > >
> > > > This comment applies to both branches of the if statement. I think
> > > > keeping it here is slightly better.
> > > >
> > > > > - QSD_NEED_PID=y $QSD \
> > > > > - --blockdev
> > > > > file,node-name=export-node,filename=$img_name,discard=unmap \
> > > > > - --export
> > > > > fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=on,allow-other=off
> > > > > \
> > > > > - &
> > > > > + if [ -n "$FUSE_OVER_IO_URING" ]; then
> > > > > + nr_cpu=$(nproc 2>/dev/null || echo 1)
> > > > > + nr_iothreads=$((nr_cpu / 2))
> > > > > + if [ $nr_iothreads -lt 1 ]; then
> > > > > + nr_iothreads=1
> > > > > + fi
> > > >
> > > > Please add a comment explaining that the purpose of this configuration
> > > > based on the number of CPUs is to test multiple IOThreads when the host
> > > > allows it, since that is a more interesting case then just 1 IOThread.
> > > > Many other configurations are possible as well, but not all of them can
> > > > be tested because the test matrix would be large.
> > > >
> > > > > +
> > > > > + iothread_args=""
> > > > > + iothread_export_args=""
> > > > > + for ((i=0; i<$nr_iothreads; i++)); do
> > > > > + iothread_args="$iothread_args --object
> > > > > iothread,id=iothread$i"
> > > > > +
> > > > > iothread_export_args="$iothread_export_args,iothread.$i=iothread$i"
> > > > > + done
> > > > > +
> > > > > + QSD_NEED_PID=y $QSD \
> > > > > + $iothread_args \
> > > > > + --blockdev
> > > > > file,node-name=export-node,filename=$img_name,discard=unmap \
> > > > > + --export
> > > > > fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=on,allow-other=off,io-uring=on$iothread_export_args
> > > > > \
> > > > > + &
> > > > > + else
> > > > > + # Usually, users would export formatted nodes. But we
> > > > > present fuse as a
> > > > > + # protocol-level driver here, so we have to leave the
> > > > > format to the
> > > > > + # client.
> > > > > + # Switch off allow-other, because in general we do not
> > > > > need it for
> > > > > + # iotests. The default allow-other=auto has the
> > > > > downside of printing a
> > > > > + # fusermount error on its first attempt if allow_other
> > > > > is not
> > > > > + # permissible, which we would need to filter.
> > > > > + QSD_NEED_PID=y $QSD \
> > > > > + --blockdev
> > > > > file,node-name=export-node,filename=$img_name,discard=unmap \
> > > > > + --export
> > > > > fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=on,allow-other=off
> > > > > \
> > > > > + &
> > > > > + fi
> > > > > pidfile="$QEMU_TEST_DIR/qemu-storage-daemon.pid"
> > > > > @@ -592,6 +613,8 @@ _rm_test_img()
> > > > > kill "${FUSE_PIDS[index]}"
> > > > > + sleep 1
> > > > > +
> > > >
> > > > What is the purpose of this sleep command?
> > > >
> > >
> > > I don’t exactly remember why. It might get stuck if there’s no sleep
> > > here. I
> > > remember we discussed this problem in earlier emails.
> >
> > The purpose needs to be understood. Otherwise there is a good chance
> > that the test will fail randomly in a continuous integration environment
> > where things sometimes take a long time due to CPU contention.
> >
> > Stefan
>
> I think the issue lies in our current approach of using df to check whether
> the FUSE mount has been unmounted.
>
> When we traced df with strace, we found that its logic for checking the
> mount point is:
> => Call mount to read the system's mount information
> => Use statfs() to get the filesystem statistics
>
> But our current test code exits with the following sequence:
> => Kill the FUSE process
> => The kernel starts cleaning up the FUSE mount point
> => df calls statfs(), which requires communication with the FUSE process But
> the FUSE process might still be cleaning up, causing the communication to
> fail
> => df then returns an error or stale information
> => Our detection logic misinterprets this and immediately deletes the
> mounted image
>
> Since we only need to check the system's mount information, we can just call
> mount and grep "$img" to verify whether the image has been successfully
> unmounted.
>
> Does it make sense?It's worth trying. Hanna wrote the existing code that uses df(1), so maybe she has thoughts on this too. I looked at waiting for FUSE_PIDS[] or using the QMP monitor to shut down cleanly. Those approaches have their own issues. Sending a `block-export-del` QMP command and waiting for it to return, followed by a `quit` QMP command should work well. But it's more complex than adjusting the existing loop and still needs a timeout. So I think the mount(8) approach is worth a shot. Stefan
signature.asc
Description: PGP signature
