[OpenWrt-Devel] [PATCH fstools] Revert "block: mount_action: handle mount/umount deps"

Rafał Miłecki zajec5 at gmail.com
Sat Dec 28 05:27:56 EST 2019

On 27.12.2019 13:25, Yousong Zhou wrote:
> On Fri, 27 Dec 2019 at 16:53, Rafał Miłecki <zajec5 at gmail.com> wrote:
>> From: Rafał Miłecki <rafal at 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

# 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'
	option	uuid	'BBBB'
	option	enabled	'1'
	option	autofs	'1'

openwrt-devel mailing list
openwrt-devel at lists.openwrt.org

More information about the openwrt-devel mailing list