On 2018/1/25 16:21, Amir Goldstein Wrote:
> On Thu, Jan 25, 2018 at 8:39 AM, zhangyi (F) <yi.zhang@xxxxxxxxxx> wrote:
>> +# Check the same mnt/dev of _check_overlay_scratch_fs, but check optionally
>> +# lower/upper/work dirs of overlay filesystem, should use together with
>> +# _require_overlay_scratch_dirs
>> +_overlay_check_scratch_dirs()
>> +{
>> + local lowerdir=$1
>> + local upperdir=$2
>> + local workdir=$3
>> +
>> + # Need to umount overlay for scratch dir check
>> + local ovl_mounted=`_is_mounted $SCRATCH_MNT`
>> + [ -z "$ovl_mounted" ] || $UMOUNT_PROG $SCRATCH_MNT
>> +
>> + _overlay_check_dirs $lowerdir $upperdir $workdir
>> + local ret=$?
>> +
>> + if [ $ret -eq 0 -a -n "$ovl_mounted" ]; then
>> + # overlay was mounted, remount besides extra mount options
>> + _overlay_scratch_mount_dirs $lowerdir $upperdir $workdir
>
> I think that 'shift 3' and passing $* to _overlay_scratch_mount_dirs
> would be good practice here.
> Especially, because extra mount options may affect fsck.overlay
> someday. Imagine that fsck.overlay ... -oindex=on may be used to
> verify consistency and rebuild index for hardlinks and -onfs_export=on may
> be used to specify consistency check and rebuild of index for all
> files and directories.
>
Good suggestion, fsck.overlay will handles these extra options sooner or later.
[..]
>> +
>> + [ "$FSTYP" = overlay ] || return 0
>> +
>> + # Base fs needs to be mounted to check overlay dirs
>> + local base_mounted=""
>> + [ -z "$base_dev" ] || \
>> + base_mounted=`_is_mounted $base_dev $OVL_BASE_FSTYP`
>
> Not that I am proposing to support -overlay run where FSTYP is not
> defined in config file, but how does this behave in such a case?
> Perhaps add || [ -z "$OVL_BASE_FSTYP" ] above?
>
If FSTYP is not defined, _is_mounted will check the wrong fs and base_mounted
becomes empty, and it's same to add [ -z "$OVL_BASE_FSTYP" ], both will lead
to double mount.
I think it's better to use _fs_type as _check_generic_filesystem does.
Thanks,
Yi.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
