On 27.12.2019 13:25, Yousong Zhou wrote:
On Fri, 27 Dec 2019 at 16:53, Rafał Miłecki <[email protected]> wrote:

From: Rafał Miłecki <[email protected]>

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
[email protected]
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to