Re: [PATCH 2/5] tests/qemu-iotests: skip 108 when FUSE is not loaded

[Daniel P . Berrangé]

On Wed, Jun 15, 2022 at 11:48:02AM -0400, John Snow wrote:
> On Wed, Jun 15, 2022 at 11:33 AM Daniel P. Berrangé <berrange@redhat.com> 
> wrote:
> >
> > On Wed, Jun 15, 2022 at 09:41:32AM -0400, John Snow wrote:
> > > On Tue, Jun 14, 2022 at 10:30 AM John Snow <jsnow@redhat.com> wrote:
> > > >
> > > > On Tue, Jun 14, 2022 at 4:59 AM Daniel P. Berrangé 
> > > > <berrange@redhat.com> wrote:
> > > > >
> > > > > On Tue, Jun 14, 2022 at 06:46:35AM +0200, Thomas Huth wrote:
> > > > > > On 14/06/2022 03.50, John Snow wrote:
> > > > > > > In certain container environments we may not have FUSE at all, so 
> > > > > > > skip
> > > > > > > the test in this circumstance too.
> > > > > > >
> > > > > > > Signed-off-by: John Snow <jsnow@redhat.com>
> > > > > > > ---
> > > > > > >   tests/qemu-iotests/108 | 6 ++++++
> > > > > > >   1 file changed, 6 insertions(+)
> > > > > > >
> > > > > > > diff --git a/tests/qemu-iotests/108 b/tests/qemu-iotests/108
> > > > > > > index 9e923d6a59f..e401c5e9933 100755
> > > > > > > --- a/tests/qemu-iotests/108
> > > > > > > +++ b/tests/qemu-iotests/108
> > > > > > > @@ -60,6 +60,12 @@ if sudo -n losetup &>/dev/null; then
> > > > > > >   else
> > > > > > >       loopdev=false
> > > > > > > +    # Check for fuse support in the host environment:
> > > > > > > +    lsmod | grep fuse &>/dev/null;
> > > > > >
> > > > > > That doesn't work if fuse has been linked statically into the 
> > > > > > kernel. Would
> > > > > > it make sense to test for /sys/fs/fuse instead?
> > > > > >
> > > > > > (OTOH, we likely hardly won't run this on statically linked kernels 
> > > > > > anyway,
> > > > > > so it might not matter too much)
> > > > >
> > > > > But more importantly 'lsmod' may not be installed in our container
> > > > > images. So checking /sys/fs/fuse avoids introducing a dep on the
> > > > > 'kmod' package.
> > > > >
> > > > > >
> > > > > > > +    if [[ $? -ne 0 ]]; then
> > > > > >
> > > > > > I'd prefer single "[" instead of "[[" ... but since we're requiring 
> > > > > > bash
> > > > > > anyway, it likely doesn't matter.
> > > > >
> > > > > Or
> > > > >
> > > > >     if  test $? != 0 ; then
> > > > >
> > > > > >
> > > > > > > +        _notrun 'No Passwordless sudo nor FUSE kernel module'
> > > > > > > +    fi
> > > > > > > +
> > > > > > >       # QSD --export fuse will either yield "Parameter 'id' is 
> > > > > > > missing"
> > > > > > >       # or "Invalid parameter 'fuse'", depending on whether there 
> > > > > > > is
> > > > > > >       # FUSE support or not.
> > > > > >
> > > >
> > > > Good suggestions, thanks!
> > > >
> > >
> > > I think I need to test against /dev/fuse instead, because /sys/fs/fuse
> > > actually exists, but because of docker permissions (etc), FUSE isn't
> > > actually usable from the child container.
> > >
> > > I wound up with this:
> > >
> > > # Check for usable FUSE in the host environment:
> > > if test ! -c "/dev/fuse"; then
> > >     _notrun 'No passwordless sudo nor usable /dev/fuse'
> > > fi
> > >
> > > Seems to work for my case here, at least, but I don't have a good
> > > sense for how broadly flexible it might be. It might be nicer to
> > > concoct some kind of NOP fuse mount instead, but I wasn't able to
> > > figure out such a command quickly.
> > >
> > > The next problem I have is actually related; test-qga (for the
> > > Centos.x86_64 run) is failing because the guest agent is reading
> > > /proc/self/mountinfo -- which contains entries for block devices that
> > > are not visible in the current container scope. I think when QGA goes
> > > to read info about these devices to populate a response, it chokes.
> > > This might be a genuine bug in QGA if we want it to tolerate existing
> > > inside of a container.
> >
> > Yes, we should fix this. Even if you don't run QGA in a container,
> > someone might configure the systemd service to harden it, by
> > restricting what /dev it is able to see and thus trigger the
> > same issue.
> 
> Naive solution: if we try to look in /sys/dev/block/%u:%u and find
> that we are unable to do so for whatever reason (ENOENT et al), just
> skip that entry for the fsinfo returned to the caller.
> 
> Does it need to be fancier than that?

/sys stuff maybe unfiltered, while /dev is restricted.

I've not looked at the QGA code for this, but conceptually I think
I would just identify where in the code errors hit, and ignore the
appropriate error conditions. The goal is to return as much info
as we reasonably can offer, given our execution environment
constraints.


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