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

[Rafał Miłecki]

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

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel