Re: [PATCH v5] tests/functional: add a vfio-user smoke test

["Daniel P. Berrangé" <berrange@redhat.com>]

On Thu, Sep 11, 2025 at 05:27:24PM +0200, Thomas Huth wrote:
>  Hi!
> 
> On 11/09/2025 16.22, John Levon wrote:
> > From: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> > 
> > Add a basic test of the vfio-user PCI client implementation.
> > 
> > Co-authored-by: John Levon <john.levon@nutanix.com>
> > Signed-off-by: Mark Cave-Ayland <mark.caveayland@nutanix.com>
> > Signed-off-by: John Levon <john.levon@nutanix.com>
> > ---
> ...
> > diff --git a/tests/functional/x86_64/test_vfio_user_client.py b/tests/functional/x86_64/test_vfio_user_client.py
> > new file mode 100755
> > index 0000000000..1e4c5bc875
> > --- /dev/null
> > +++ b/tests/functional/x86_64/test_vfio_user_client.py

> > +class VfioUserClient(QemuSystemTest):
> > +
> > +    ASSET_REPO = 'https://github.com/mcayland-ntx/libvfio-user-test'
> 
> Not sure whether that indirection works with the asset pre-caching
> mechanism? Daniel, could you comment on that?

It should be fine - the asset caching loads the class and
at that time python will have done the substitution.

> 
> > +    ASSET_KERNEL = Asset(
> > +        f'{ASSET_REPO}/raw/refs/heads/main/images/bzImage',
> > +        '40292fa6ce95d516e26bccf5974e138d0db65a6de0bc540cabae060fe9dea605'
> > +    )
> > +
> > +    ASSET_ROOTFS = Asset(
> > +        f'{ASSET_REPO}/raw/refs/heads/main/images/rootfs.ext2',
> > +        'e1e3abae8aebb8e6e77f08b1c531caeacf46250c94c815655c6bbea59fc3d1c1'
> > +    )



> > +    def prepare_images(self):
> > +        """Download the images for the VMs."""
> > +        self.kernel_path = self.ASSET_KERNEL.fetch()
> > +        self.rootfs_path = self.ASSET_ROOTFS.fetch()

Just put this inline, it doesn't seem this method now we removed
the extra copying logic

> > +
> > +    def configure_server_vm_args(self, server_vm, sock_path):
> > +        """
> > +        Configuration for the server VM. Set up virtio-serial device backed by
> > +        the given socket path.
> > +        """
> > +        server_vm.add_args('-kernel', self.kernel_path)
> > +        server_vm.add_args('-append', 'console=ttyS0 root=/dev/sda')
> > +        server_vm.add_args('-drive',
> > +            f"file={self.rootfs_path},if=ide,format=raw,id=drv0")
> > +        server_vm.add_args('-snapshot')
> > +        server_vm.add_args('-chardev',
> > +            f"socket,id=sock0,path={sock_path},telnet=off,server=on,wait=off")
> > +        server_vm.add_args('-device', 'virtio-serial')
> > +        server_vm.add_args('-device',
> > +            'virtserialport,chardev=sock0,name=org.fedoraproject.port.0')
> > +
> > +    def configure_client_vm_args(self, client_vm, sock_path):
> > +        """
> > +        Configuration for the client VM. Point the vfio-user-pci device to the
> > +        socket path configured above.
> > +        """
> > +
> > +        client_vm.add_args('-kernel', self.kernel_path)
> > +        client_vm.add_args('-append', 'console=ttyS0 root=/dev/sda')
> > +        client_vm.add_args('-drive',
> > +            f'file={self.rootfs_path},if=ide,format=raw,id=drv0')
> > +        client_vm.add_args('-snapshot')
> > +        client_vm.add_args('-device',
> > +            '{"driver":"vfio-user-pci",' +
> > +            '"socket":{"path": "%s", "type": "unix"}}' % sock_path)
> > +
> > +    def setup_vfio_user_pci_server(self, server_vm):
> > +        """
> > +        Start the libvfio-user server within the server VM, and arrange
> > +        for data to shuttle between its socket and the virtio serial port.
> > +        """
> > +        wait_for_console_pattern(self, 'login:', None, server_vm)
> > +        exec_command_and_wait_for_pattern(self, 'root', '#', None, server_vm)
> > +
> > +        exec_command_and_wait_for_pattern(self,
> > +            'gpio-pci-idio-16 -v /tmp/vfio-user.sock >/var/tmp/gpio.out 2>&1 &',
> > +            '#', None, server_vm)
> > +        # wait for libvfio-user to initialize properly
> > +        exec_command_and_wait_for_pattern(self, 'sleep 5', '#', None, server_vm)
> 
> Could the sleep be avoided? ... it's still a race condition (even if it's
> unlikely when you wait for 5 seconds), and always sleeping 5 seconds slows
> down the test quite a bit ...
> 
> Could you maybe poll something instead, e.g. output of "dmesg" or something
> in the file system? (sorry, I don't have any clue about vfio-user, so I
> don't know any better suggestions)
> 
> > +        exec_command_and_wait_for_pattern(self,
> > +            'socat UNIX-CONNECT:/tmp/vfio-user.sock /dev/vport0p1,ignoreeof ' +
> > +            ' &', '#', None, server_vm)
> > +
> > +    def test_vfio_user_pci(self):
> > +        self.prepare_images()
> 
> Please move the "prepare_images" after the set_machine() and
> require_device() calls. Reason: set_machine() and require_device() could
> skip the test if it's not available in the qemu binary, so in that case you
> don't want to try to fetch the assets first.
> 
> > +        self.set_machine('pc')
> > +        self.require_device('virtio-serial')
> > +        self.require_device('vfio-user-pci')
> > +
> > +        sock_dir = self.socket_dir()
> > +        socket_path = sock_dir.name + '/vfio-user.sock'
> 
> Better use os.path.join() instead of hard-coding slashes.
> 
>  Thanks,
>   Thomas
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|