Re: [PATCH for-9.0 v3 2/2] qtest/virtio-9p-test.c: remove g_test_slow() gate

[Daniel P . Berrangé]

On Wed, Apr 17, 2024 at 01:52:24PM +0200, Christian Schoenebeck wrote:
> On Wednesday, April 17, 2024 1:16:02 AM CEST Daniel Henrique Barboza wrote:
> > 
> > On 4/16/24 16:54, Michael Tokarev wrote:
> > > 27.03.2024 17:20, Daniel Henrique Barboza :
> > >> Commit 558f5c42ef gated the local tests with g_test_slow() to skip them
> > >> in 'make check'. The reported issue back then was this following CI
> > >> problem:
> > >>
> > >> https://lists.nongnu.org/archive/html/qemu-devel/2020-11/msg05510.html
> > >>
> > >> This problem ended up being fixed after it was detected with the
> > >> recently added risc-v machine nodes [1]. virtio-9p-test.c is now
> > >> creating and removing temporary dirs for each test run, instead of
> > >> creating a single dir for the entire qos-test scope.
> > >>
> > >> We're now able to run these tests with 'make check' in the CI, so let's
> > >> go ahead and re-enable them.
> > >>
> > >> This reverts commit 558f5c42efded3e0d0b20a90bce2a9a14580d824.
> > >>
> > >> [1] https://mail.gnu.org/archive/html/qemu-devel/2024-03/msg05807.html
> > > 
> > > This makes tests being unable to complete on a tmpfs.  It looks like
> > > 9pfs tests needs another tweak here.
> > > 
> > > # starting QEMU: exec ./qemu-system-x86_64 -qtest 
> > > unix:/tmp/qtest-798502.sock -qtest-log /dev/null -chardev 
> > > socket,path=/tmp/qtest-798502.qmp,id=char0 -mon 
> > > chardev=char0,mode=control -display none -audio none -M pc  -fsdev 
> > > local,id=fsdev0,path='/tmp/q/master/qtest-9p-local-9LHRL2',security_model=mapped-xattr
> > >  -device virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest
> > > Received response 7 (RLERROR) instead of 73 (RMKDIR)
> > > Rlerror has errno 95 (Operation not supported)
> > > **
> > > ERROR:../../../build/qemu/master/tests/qtest/libqos/virtio-9p-client.c:275:v9fs_req_recv:
> > >  assertion failed (hdr.id == id): (7 == 73)
> > > 
> > > This is when I build it on /tmp/ which is a tmpfs.  When I build
> > > it on a real filesystem, it works fine.
> > > 
> > > Apparently xattrs aren't supported on a tmpfs.
> > 
> > Hmmm not sure how to proceed here since I'm not a 9p expert by any means. 
> > I'll
> > let Christian decide what to do.
> > 
> > If we can't figure it out we might need to re-introduce the gate again. 
> > Thanks,
> 
> It's not that tmpfs exactly doesn't support xattrs. It supports the trusted.*
> and security.* namespaces since 2011, so tmpfs was limited to those two. For
> the 9p 'local' backend however we also need the user.* namespace which was
> just added in Linux 6.6 last year (commit 2daf18a).
> 
> Unfortunately the respective kernel option TMPFS_XATTR is still off by default
> (linux/fs/Kconfig).
> 
> Back then, when we added that 'slow' gate for the 9p 'local' tests, things
> were a bit different. They simply did not run in the gitlab pipeline (for
> reasons described above). Now they do.
> 
> So obviously it would make sense to preserve these tests for the gitlab
> pipeline this time, e.g. by skipping these tests only if the underlying test
> directory does not support *.user xattrs. I'm just not sure yet where exactly
> such kind of *active* check would fit best into the glib test layout, as this
> can be a bit tricky with glib

You should run a method which checks ability to use '.user' xattrs, and
if it reports failure, then skip calling g_test_add. IOW, you can put
the xattr test in the same place as the old g_test_slow() check was.


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 :|