| Message ID | 20220614015044.2501806-3-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/5] tests/qemu-iotests: hotfix for 307, 223 output | expand |
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) > + if [[ $? -ne 0 ]]; then I'd prefer single "[" instead of "[[" ... but since we're requiring bash anyway, it likely doesn't matter. > + _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. Thomas
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. > > Thomas > With regards, Daniel
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! --js
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. --js
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. With regards, Daniel
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? --js
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
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; + if [[ $? -ne 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.
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(+)