| Message ID | 698CC47A-DC4A-4705-B112-6AAC36A088A4@icloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 3, 2018 at 4:25 PM, Chengguang Xu <cgxu519@icloud.com> wrote: >> >> 在 2018年1月3日，下午8:58，Amir Goldstein <amir73il@gmail.com> 写道： >> >> On Wed, Jan 3, 2018 at 2:44 PM, Chengguang Xu <cgxu519@icloud.com> wrote: >>> >> [...] >>> In order to add overlay support in some requirement checks like _require_metadata_journaling, >>> I think it’s better save underlying filesystem type to $OVL_BASE_FSTYP and doing proper check based >>> on it. >>> >>> Currently $OVL_BASE_FSTYP and $FSTYP all set to “overlay”, Is there any specific reason for it? >>> >> >> In _overlay_config_override(): >> # Config file may specify base fs type, but we obay -overlay flag >> export OVL_BASE_FSTYP="$FSTYP" >> export FSTYP=overlay >> >> So either your setup is wrong or there is a bug. >> $OVL_BASE_FSTYP *should* contain the base fs type, >> but only if you defined FSTYPE in your config file when running ./check -overlay > > Yeah, I didn’t specify by hand in config file and just supposed to detect automatically. > I didn’t check very carefully but seems just slight modification like below could let it > support auto detection. > Sorry, I do not wish invest time to review this change because: 1. I don't see the need to support auto detect of base fs type (feel free to explain) 2. I ran a lot of test to sanitize the new overlay config option with many configurations and this adds a lot more variants. I you wish to push this forward and have a good claim for the need, please specify which tests you ran to sanitize your change before requesting review. If you wish to contribute to the xfstests overlay infrastructure there are other features that would bring more gain IMO, for example: - Support mkfs and MKFS_OPTIONS for base fs - Try to increase the number of generic tests that can be run with basefs+overlay similar to your effort with the shutdown group tests Thanks! Amir. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
在 2018年1月3日，下午10:54，Amir Goldstein <amir73il@gmail.com> 写道： > > On Wed, Jan 3, 2018 at 4:25 PM, Chengguang Xu <cgxu519@icloud.com> wrote: >>> >>> 在 2018年1月3日，下午8:58，Amir Goldstein <amir73il@gmail.com> 写道： >>> >>> On Wed, Jan 3, 2018 at 2:44 PM, Chengguang Xu <cgxu519@icloud.com> wrote: >>>> >>> [...] >>>> In order to add overlay support in some requirement checks like _require_metadata_journaling, >>>> I think it’s better save underlying filesystem type to $OVL_BASE_FSTYP and doing proper check based >>>> on it. >>>> >>>> Currently $OVL_BASE_FSTYP and $FSTYP all set to “overlay”, Is there any specific reason for it? >>>> >>> >>> In _overlay_config_override(): >>> # Config file may specify base fs type, but we obay -overlay flag >>> export OVL_BASE_FSTYP="$FSTYP" >>> export FSTYP=overlay >>> >>> So either your setup is wrong or there is a bug. >>> $OVL_BASE_FSTYP *should* contain the base fs type, >>> but only if you defined FSTYPE in your config file when running ./check -overlay >> >> Yeah, I didn’t specify by hand in config file and just supposed to detect automatically. >> I didn’t check very carefully but seems just slight modification like below could let it >> support auto detection. >> > > Sorry, I do not wish invest time to review this change because: > 1. I don't see the need to support auto detect of base fs type (feel > free to explain) The reason of supporting auto detection maybe is the same as xfstests for local filesystem. For some people who need to frequently change testing filesystem, they would prefer to automatically recognize testing filesystem than modifying config file again and again. > 2. I ran a lot of test to sanitize the new overlay config option with many > configurations and this adds a lot more variants. I you wish to push this > forward and have a good claim for the need, please specify which tests > you ran to sanitize your change before requesting review. > > If you wish to contribute to the xfstests overlay infrastructure there > are other features > that would bring more gain IMO, for example: > - Support mkfs and MKFS_OPTIONS for base fs > - Try to increase the number of generic tests that can be run with > basefs+overlay > similar to your effort with the shutdown group tests OK，let’s do these first. Thanks, Chengguang. -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/check b/check index f8db3cd..e460424 100755 --- a/check +++ b/check @@ -265,7 +265,7 @@ while [ $# -gt 0 ]; do -nfs) FSTYP=nfs ;; -glusterfs) FSTYP=glusterfs ;; -cifs) FSTYP=cifs ;; - -overlay) FSTYP=overlay; export OVERLAY=true ;; + -overlay) export OVERLAY=true ;; -pvfs2) FSTYP=pvfs2 ;; -tmpfs) FSTYP=tmpfs ;; -ubifs) FSTYP=ubifs ;; diff --git a/common/config b/common/config index d0fbfe5..e21d8ee 100644 --- a/common/config +++ b/common/config @@ -530,7 +530,9 @@ _overlay_config_override() [ -b "$TEST_DEV" ] || return 0 # Config file may specify base fs type, but we obay -overlay flag - export OVL_BASE_FSTYP="$FSTYP" + if [ -z $OVL_BASE_FSTYP ]; then + export OVL_BASE_FSTYP="$FSTYP" + fi export FSTYP=overlay # Store original base fs vars @@ -691,13 +693,6 @@ get_next_config() { _check_device SCRATCH_LOGDEV optional $SCRATCH_LOGDEV fi - # Override FSTYP from config when running ./check -overlay - # and maybe override base fs TEST/SCRATCH_DEV with overlay base dirs. - # We need to do this *after* default mount options are set by base FSTYP - # and *after* SCRATCH_DEV is deduced from SCRATCH_DEV_POOL - if [ "$OVERLAY" == "true" -o "$FSTYP" == "overlay" ]; then - _overlay_config_override - fi } if [ -z "$CONFIG_INCLUDED" ]; then @@ -711,6 +706,15 @@ if [ -z "$CONFIG_INCLUDED" ]; then [ ! -z "$TEST_DEV" ]; then FSTYP=`blkid -c /dev/null -s TYPE -o value $TEST_DEV` fi + + # Override FSTYP from config when running ./check -overlay + # and maybe override base fs TEST/SCRATCH_DEV with overlay base dirs. + # We need to do this *after* default mount options are set by base FSTYP + # and *after* SCRATCH_DEV is deduced from SCRATCH_DEV_POOL + if [ "$OVERLAY" == "true" -o "$FSTYP" == "overlay" ]; then + _overlay_config_override + fi + FSTYP=${FSTYP:=xfs} export FSTYP [ -z "$MOUNT_OPTIONS" ] && _mount_opts