about test mount/rlimit

about test mount/rlimit

[Ruediger Meier]

Hi,

Some questions regarding test mount/rlimit which runs only if /etc/mtab 
is a file.

This is the umount subtest:

.......
ts_init_subtest "umount"
[ -d "$TS_MOUNTPOINT" ] || mkdir -p $TS_MOUNTPOINT
$TS_CMD_MOUNT $DEVICE $TS_MOUNTPOINT &> /dev/null
OLD_SUM=$(mtab_checksum)
(
        ulimit -f 1
        $TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
) &> /dev/null
NEW_SUM=$(mtab_checksum)
$TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
[ $NEW_SUM = $OLD_SUM ] && echo "OK: mtab unmodified by umount" >> 
$TS_OUTPUT
ts_finalize_subtest
...........

I do not fully understand what is expected. Obviously the first "umount" 
should NOT modify /etc/mtab. But should it umount or not, or doesn't 
matter? Why we call a second umount then?

The problem is that in case that the first umount works we are leaving 
the test with a broken mtab file. This is not nice and also problematic 
because later tests may find a mounted /dev/loop0 in /etc/mtab.

Should we restore the original mtab? I guess it's a bit unsafe to copy 
back a backed up one. Or is there a way to "repair/sync" 
from /proc/mounts.?

cu,
Rudi

Re: about test mount/rlimit

[Karel Zak]

On Wed, Apr 01, 2015 at 02:04:27PM +0200, Ruediger Meier wrote:
> ts_init_subtest "umount"
> [ -d "$TS_MOUNTPOINT" ] || mkdir -p $TS_MOUNTPOINT
> $TS_CMD_MOUNT $DEVICE $TS_MOUNTPOINT &> /dev/null
> OLD_SUM=$(mtab_checksum)
> (
>         ulimit -f 1
>         $TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
> ) &> /dev/null
> NEW_SUM=$(mtab_checksum)
> $TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
> [ $NEW_SUM = $OLD_SUM ] && echo "OK: mtab unmodified by umount" >> 
> $TS_OUTPUT
> ts_finalize_subtest
> ...........
> 
> I do not fully understand what is expected. Obviously the first "umount" 
> should NOT modify /etc/mtab. But should it umount or not, or doesn't 
> matter? Why we call a second umount then?

I guess the second umount is just copy & past (from mount test) and
it's there to be sure that the device has been really unmounted (and
this is unnecessary because ts_device_deinit() calls umount too).

> The problem is that in case that the first umount works we are leaving
> the test with a broken mtab file. This is not nice and also problematic 
> because later tests may find a mounted /dev/loop0 in /etc/mtab.

It would be probably better to replace the second umount with "umount
--fake" to only remove entry from mtab.

> Should we restore the original mtab? I guess it's a bit unsafe to copy 
> back a backed up one. Or is there a way to "repair/sync" 
> from /proc/mounts.?

Don't use /proc/mounts. It's bad idea as the files maybe very different.

use the --fake or you can try to redirect libmount to use non-standard
mtab by env. variable LIBMOUNT_MTAB=, but it will increase complexity
of the test.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: about test mount/rlimit

[Ruediger Meier]

On Thursday 02 April 2015, Karel Zak wrote:
> On Wed, Apr 01, 2015 at 02:04:27PM +0200, Ruediger Meier wrote:
> > ts_init_subtest "umount"
> > [ -d "$TS_MOUNTPOINT" ] || mkdir -p $TS_MOUNTPOINT
> > $TS_CMD_MOUNT $DEVICE $TS_MOUNTPOINT &> /dev/null
> > OLD_SUM=$(mtab_checksum)
> > (
> >         ulimit -f 1
> >         $TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
> > ) &> /dev/null
> > NEW_SUM=$(mtab_checksum)
> > $TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
> > [ $NEW_SUM = $OLD_SUM ] && echo "OK: mtab unmodified by umount" >>
> > $TS_OUTPUT
> > ts_finalize_subtest
> > ...........
> >
> > I do not fully understand what is expected. Obviously the first
> > "umount" should NOT modify /etc/mtab. But should it umount or not,
> > or doesn't matter? Why we call a second umount then?
>
> I guess the second umount is just copy & past (from mount test) and

Ah, now it makes sense. So the first umount should work and we could
check "ismounted" too.
BTW could it be that in past "umount" always removed the mtab entry
like "--fake" still does for umounted devices?

> it's there to be sure that the device has been really unmounted (and
> this is unnecessary because ts_device_deinit() calls umount too).


> > The problem is that in case that the first umount works we are
> > leaving the test with a broken mtab file. This is not nice and also
> > problematic because later tests may find a mounted /dev/loop0 in
> > /etc/mtab.
>
> It would be probably better to replace the second umount with "umount
> --fake" to only remove entry from mtab.

Ah that's good. I'll send a patch with some other fixes later.
The umount part would look like this: 

 	$TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
 ) &> /dev/null
 NEW_SUM=$(mtab_checksum)
-$TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
 [ $NEW_SUM = $OLD_SUM ] && echo "OK: mtab unmodified by umount" >> $TS_OUTPUT
+ts_is_mounted $DEVICE && echo "FAIL: $DEVICE is still mounted" >> $TS_OUTPUT
+# repair /etc/mtab
+$TS_CMD_UMOUNT --fake $TS_MOUNTPOINT &> /dev/null
+ts_is_mounted $DEVICE && ts_die "$DEVICE is still mounted"
 ts_finalize_subtest

> > Should we restore the original mtab? I guess it's a bit unsafe to
> > copy back a backed up one. Or is there a way to "repair/sync"
> > from /proc/mounts.?
>
> Don't use /proc/mounts. It's bad idea as the files maybe very
> different.
>
> use the --fake or you can try to redirect libmount to use
> non-standard mtab by env. variable LIBMOUNT_MTAB=, but it will
> increase complexity of the test.
>
>     Karel

Re: about test mount/rlimit

[Ruediger Meier]

On Thursday 02 April 2015, Ruediger Meier wrote:
> Ah that's good. I'll send a patch with some other fixes later.

https://github.com/karelzak/util-linux/pull/188

cu,
Rudi

Re: about test mount/rlimit

[Karel Zak]

On Thu, Apr 02, 2015 at 12:05:20PM +0200, Ruediger Meier wrote:
> On Thursday 02 April 2015, Karel Zak wrote:
> > On Wed, Apr 01, 2015 at 02:04:27PM +0200, Ruediger Meier wrote:
> > > ts_init_subtest "umount"
> > > [ -d "$TS_MOUNTPOINT" ] || mkdir -p $TS_MOUNTPOINT
> > > $TS_CMD_MOUNT $DEVICE $TS_MOUNTPOINT &> /dev/null
> > > OLD_SUM=$(mtab_checksum)
> > > (
> > >         ulimit -f 1
> > >         $TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
> > > ) &> /dev/null
> > > NEW_SUM=$(mtab_checksum)
> > > $TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
> > > [ $NEW_SUM = $OLD_SUM ] && echo "OK: mtab unmodified by umount" >>
> > > $TS_OUTPUT
> > > ts_finalize_subtest
> > > ...........
> > >
> > > I do not fully understand what is expected. Obviously the first
> > > "umount" should NOT modify /etc/mtab. But should it umount or not,
> > > or doesn't matter? Why we call a second umount then?
> >
> > I guess the second umount is just copy & past (from mount test) and
> 
> Ah, now it makes sense. So the first umount should work and we could
> check "ismounted" too.

yes

> BTW could it be that in past "umount" always removed the mtab entry
> like "--fake" still does for umounted devices?

git show v2.13:mount/umount.c

       if (!nomtab &&
             (umnt_err == 0 || umnt_err == EINVAL || umnt_err == ENOENT)) {
                 update_mtab (node, NULL);
       }


so yes, but for me it's little bit fragile (for example EINVAL means
wrong mount options in same situations, unreachable mountpoint maybe
in situation when it's hidden by another mount and with namespaces it
does not make sense at all).

Thanks God that mtab is dead.

> Ah that's good. I'll send a patch with some other fixes later.
> The umount part would look like this: 
> 
>  	$TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
>  ) &> /dev/null
>  NEW_SUM=$(mtab_checksum)
> -$TS_CMD_UMOUNT $TS_MOUNTPOINT &> /dev/null
>  [ $NEW_SUM = $OLD_SUM ] && echo "OK: mtab unmodified by umount" >> $TS_OUTPUT
> +ts_is_mounted $DEVICE && echo "FAIL: $DEVICE is still mounted" >> $TS_OUTPUT
> +# repair /etc/mtab
> +$TS_CMD_UMOUNT --fake $TS_MOUNTPOINT &> /dev/null
> +ts_is_mounted $DEVICE && ts_die "$DEVICE is still mounted"
>  ts_finalize_subtest

Seems good.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com