Re: [PATCH 4/4] iotests: add tests for FUSE-over-io_uring

[Brian Song <hibriansong@gmail.com>]

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 <kwolf@redhat.com>
>>>> Suggested-by: Stefan Hajnoczi <stefanha@redhat.com>
>>>> Signed-off-by: Brian Song <hibriansong@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?

Brian