On Sat, 28 Dec 2019 at 18:27, Rafał Miłecki <zaj...@gmail.com> wrote:
>
> On 27.12.2019 13:25, Yousong Zhou wrote:
> > On Fri, 27 Dec 2019 at 16:53, Rafał Miłecki <zaj...@gmail.com> wrote:
> >>
> >> From: Rafał Miłecki <ra...@milecki.pl>
> >>
> >> This reverts commit 32c3126b2f0464106d74317336b6aef1d7d5f82f.
> >>
> >> Internal list of devices guarantees some basic sorting (by device type
> >> and then a name) but nothing more. In particular it's not guaranteed
> >> (and it's actually quite uncommon) that all preceding entries are parent
> >> devices.
> >>
> >> Mounting all preceding devices may easily result in unrelated mounts.
> >> They can fail easily basically breaking original mounting attempt, e.g.:
> >>
> >> daemon.err blockd: kernel is requesting a mount -> sda2
> >> daemon.err block: /dev/sda1 is already mounted on /tmp/run/blockd/sda1
> >> daemon.err block: autofs: "add" action has failed: -1
> >> daemon.err blockd: failed to run block. add/sda2
> >
> > Sorry for the inconvenience. But the error (regression) should be
> > caused by 2f2a09ad ("block: mount_device: err log only when mp
> > deviates from spec"). m->target is expected to match the actual mount
> > point only when it is not managed by blockd (m->autofs).
> >
> > Please see if the following patch works. We can also check if m is
> > managed by autofs and m->target a symlink whose target matches mp, but
> > that's a bit overkill.
> >
> > --- a/block.c
> > +++ b/block.c
> > @@ -1100,7 +1100,7 @@ static int mount_device(struct device *dev, int type)
> >
> > mp = find_mount_point(pr->dev);
> > if (mp) {
> > - if (m && m->type == TYPE_MOUNT && strcmp(m->target, mp)) {
> > + if (m && !m->autofs && m->type == TYPE_MOUNT &&
> > strcmp(m->target, mp)) {
> > ULOG_ERR("%s is already mounted on %s\n", pr->dev,
> > mp);
> > err = -1;
> > } else
> >
>
> You're right about that error and your diff indeed fixes that:
> /dev/sda1 is already mounted on /tmp/run/blockd/sda1
> for me. It still doesn't fix mounting unneeded devices though.
>
>
> Please check this:
>
> # mount | grep "/dev/sd"
>
> # ls /var/run/blockd/sdb2
> b.txt
>
> # mount | grep "/dev/sd"
> /dev/sda1 on /tmp/run/blockd/sda1 type vfat
> (rw,relatime,fmask=0000,dmask=0000,allow_utime=0022,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro)
> /dev/sda2 on /tmp/run/blockd/sda2 type vfat
> (rw,relatime,fmask=0000,dmask=0000,allow_utime=0022,codepage=437,iocharset=iso8859-1,shortname=mixed,errors=remount-ro)
> /dev/sdb1 on /tmp/run/blockd/sdb1 type fuseblk
> (rw,relatime,user_id=0,group_id=0,allow_other,blksize=4096)
> /dev/sdb2 on /tmp/run/blockd/sdb2 type fuseblk
> (rw,relatime,user_id=0,group_id=0,allow_other,blksize=4096)
>
> As you can see, accessing "sdb2" partition resulted in mounting 3 other
> partitions that I don't need at all. Including spinning 1 unused disk!
>
>
> >> If some dependency handling is required it should be implemented
> >> explicitly as current solution isn't reliable and it breaks autofs when
> >> using multiple devices (partitions).
> >>
> >
> > Dependencies are directly implied by mount target as specified in the
> > uci config file. This relationship is inherently there. E.g.
> >
> > 1. mount target /mnt/a
> > 2. mount target /mnt/a/b
> >
> > Then "1" must mount before "2". "2" before "1" is not practically
> > useful in any way.
>
> That relationship/dependency isn't directly parsed by fstools in any
> way. By making it explicit I thought of something like:
>
> config 'mount' '/dev/vdc'
> option target '/mnt'
> option uuid 'AAAA'
> option enabled '1'
> option autofs '1'
>
> config 'mount' '/dev/vdb'
> option parent '/dev/vdc/
> option target '/mnt/s'
The point is that this "parent" relationship is already implied. This
extra uci option does add more information.
> That relationship/dependency isn't directly parsed by fstools in any
> way.
That devces list was modified to be ordered primarily by length of
mount target was for this (fb0700f0 "block: support hierarchical
mount/umount"). But it's not tight/precise enough to only mount
direct dependencies, but all enabled mounts whose length smaller than
itself.
I made a patch [1] few days ago trying to amend that but didn't yet
have time to test it. Now I checked it again, that "target" defaults
to "/mnt/<dev>" and anon_mount=1 also need to be taken into
consideration. Nevertheless, this dependency tracking mechanism still
needs to be handled by the program itself, not extra knobs.
[1]
https://github.com/yousong/fstools/commit/0eab853a7857c967efbc6fb7a6e0ba20e5dd3aba
Regards,
yousong
> option uuid 'BBBB'
> option enabled '1'
> option autofs '1'
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
