Received-SPF: softfail (google.com: domain of transitioning linux-ext4-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   Shinichiro Kawasaki <shinichiro.kawasaki@wdc.com>
To:     "Darrick J. Wong" <djwong@kernel.org>
CC:     "fstests@vger.kernel.org" <fstests@vger.kernel.org>,
        "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
        "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>,
        "linux-ext4@vger.kernel.org" <linux-ext4@vger.kernel.org>,
        Naohiro Aota <Naohiro.Aota@wdc.com>,
        Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
        Damien Le Moal <damien.lemoal@opensource.wdc.com>
Subject: Re: [PATCH 5/7] common: rename _filter_mkfs to _xfs_filter_mkfs
Thread-Topic: [PATCH 5/7] common: rename _filter_mkfs to _xfs_filter_mkfs
Thread-Index: AQHYG+/CBPelkf6NS0GNBGeElFDeDqyKZC0AgAACzICAAHx7gA==
Date:   Wed, 9 Feb 2022 08:18:50 +0000
Message-ID: <20220209081849.32avg2g6x6bc7juj@shindev>
References: <20220207065541.232685-1-shinichiro.kawasaki@wdc.com>
 <20220207065541.232685-6-shinichiro.kawasaki@wdc.com>
 <20220209004316.GE8288@magnolia> <20220209005317.GE8338@magnolia>
In-Reply-To: <20220209005317.GE8338@magnolia>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?GxeJO/fcNjlAQCtGvuVKdq8NPpqCyCelBoFjBSvUdqK2iY+p1Gu7mFVAexRv?=
 =?us-ascii?Q?BVBtWlQs/y5yOnAjnwg5FOGY7YYhROpWpNKHP6KQUpHmX1QWok81OMxNXrbR?=
 =?us-ascii?Q?y4+GGvKov0NbqOIdRIbydYQ/aFWuR04ZIbg8cs/DdsVNwYnsbRre7+xNHXKM?=
 =?us-ascii?Q?tjzgZQlOgpZO04E6GZ3bxjtlhbxHcRkwk9iIDlYCKBw6R2LnnPpGqKPVThv+?=
 =?us-ascii?Q?ERuu9ScJkP1M20Pgxs1r8pBL45Av7ppouw6a00VyaxLZmMVbXE90YNnETjf0?=
 =?us-ascii?Q?qHKcGgII2FcyyQcgUL0zn6pylJNMIiY5jboT9wZ4qag2UXm05YbCjOEJX/SH?=
 =?us-ascii?Q?XbCJKrFnwN1t8lmrvOF+XDdCO2VOPZbuHNU3H4fCY7rKo8M87zNFg08t+L7X?=
 =?us-ascii?Q?mh7q54BeZwbjJMe1SELhIt1z7ikTQWH1vKpHHWpgDSD5YaSwhsTRc9FJTddj?=
 =?us-ascii?Q?1Ajt5SUG6JB5ni3xQFpZOasSHnmGjCVp8PnCIdH/VQyBG4tM/RpmkeAbpqmj?=
 =?us-ascii?Q?4iraRzsEbUWp41r1gB+UtFNfu7NVcfREWBuFn169oAjtds4qWEbLAeC/KVqr?=
 =?us-ascii?Q?kJ2gyWczDgy0cFK8w31tr9UhOJmtZ+e6R4xfsS69M1VriFdRekDD1igh4sUm?=
 =?us-ascii?Q?lkXxGxfsUK61HadornLkHX0zilNFcQeTwzguwI3lmGM8AS2IiuiwHHDbEFoe?=
 =?us-ascii?Q?OEnfcz6urHFwa2yFpmSJK1u12SUv/UhTqpwLTEBhUw1goLvtT3TOJZPbVVov?=
 =?us-ascii?Q?qv9jmd9dbAk0t0uoTVKRjStHwOwCycBg7NDlxflmY8QA9tuHqMELAI4B47ei?=
 =?us-ascii?Q?Q7+4rb3M2SXtnoJVyhTMPeNvWxUnLw8wWXCa2B6PGaao0CWCPlfDyLCBhE9b?=
 =?us-ascii?Q?KVQRdVsAKjmYfVVwnd8tLv7wcbMmjPsIfcgkS3rgsr3YJOOl6b1b5mwFvyvo?=
 =?us-ascii?Q?csf66THcITgeukR34CsqkgiunBvtZ9yT5KdhDXQXhd3O9EBirJnYws1CUvM0?=
 =?us-ascii?Q?p48OA5OXiGVO3dmx/icAWZbLEXuc4+ZS3TucEw/3UUi7etsV4wbboZcchD63?=
 =?us-ascii?Q?5CPXRvqw+0dSuXYbteHRlExpb++yUtNY7nB2RH/i78SfnGml516zqmJIrCVy?=
 =?us-ascii?Q?qyakPfr5bpygxQWgO2mrbRuuaeYkoWkgDnUPaBYhrSsm9ABX+DAoRReWsxlj?=
 =?us-ascii?Q?3ybBFH5mFe3HIX8LMPH1rAfJZQ7CxeZcvEkwL1o+Iwbhn8kYe5nSTYdzaQU+?=
 =?us-ascii?Q?ufsq8Fi8gkXYQ7pmEQ6aN/tKreyZoikiTVMMF1WpiaqdjbFZ+uuLzcwcqxRQ?=
 =?us-ascii?Q?VxcFplHTTlqHAYpoCzi30xK12jcsgTk15XBVQvghip2Hv3nxWA7YKGbqycQi?=
 =?us-ascii?Q?DhTnp1rkrCRUFRhPcRH5uZKLivVlARD3R9GmoqRMKwFHQGugR/YSZebwf5wf?=
 =?us-ascii?Q?MhAiVFQmZuvLNNTce1QYCZwdF0lLhBpzUmk4PGYVH9bv1KRn8XKhd5iLvyor?=
 =?us-ascii?Q?zwYsq9zY/+0A8DWd1Vjm8Q2pgeXrd9TqV1DgoJAM2pInwXYpcv9XnzQCRDMq?=
 =?us-ascii?Q?g+P1PRaJemeKp7Fs5+7fHoGavoYQZxwz1XLyHEYh8EpMF6dXdlxKovBVLCfD?=
 =?us-ascii?Q?N/65zjVAib8fsqwAcWCkX7vGkF2UJlzZHTvSq0bJheVTxDVvX3kTwq4yA8C9?=
 =?us-ascii?Q?xesVisM4J+7wlCsgdjDoYx2n+k0=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2472D23298DD754A9C5984E92A7EAF85@namprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: wdc.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM8PR04MB8037.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 208515c7-7138-47e9-b6a7-08d9eba4cdaf
X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Feb 2022 08:18:50.6707
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: AV7d3naGLdfW29S66NTYZnR1WktizzxLaJaD0refmR8mSikAxZ8VnmUSNvspF7Hw1AvR8g8DYEKGKk+i6g0fSkemgTlooNK3sFldKI58dso=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR04MB6017
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RDNS_NONE,
	SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-ext4.vger.kernel.org>
X-Mailing-List: linux-ext4@vger.kernel.org

On Feb 08, 2022 / 16:53, Darrick J. Wong wrote:
> On Tue, Feb 08, 2022 at 04:43:16PM -0800, Darrick J. Wong wrote:
> > On Mon, Feb 07, 2022 at 03:55:39PM +0900, Shin'ichiro Kawasaki wrote:
> > > The helper function works only for xfs and used only for xfs except
> > > generic/204. Rename the function to clearly indicate that the functio=
n
> > > is only for xfs.
> > >=20
> > > Suggested-by: Naohiro Aota <naohiro.aota@wdc.com>
> > > Signed-off-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com>
> >=20
> > <snip the diffstat>
> >=20
> > > diff --git a/common/attr b/common/attr
> > > index 35682d7c..964c790a 100644
> > > --- a/common/attr
> > > +++ b/common/attr
> > > @@ -13,7 +13,7 @@ _acl_get_max()
> > >  		# CRC format filesystems have much larger ACL counts. The actual
> > >  		# number is into the thousands, but testing that meany takes too
> > >  		# long, so just test well past the old limit of 25.
> > > -		$XFS_INFO_PROG $TEST_DIR | _filter_mkfs > /dev/null 2> $tmp.info
> > > +		$XFS_INFO_PROG $TEST_DIR | _xfs_filter_mkfs > /dev/null 2> $tmp.in=
fo
> > >  		. $tmp.info
> > >  		rm $tmp.info
> > >  		if [ $_fs_has_crcs -eq 0 ]; then
> > > diff --git a/common/filter b/common/filter
> > > index c3db7a56..24fd0650 100644
> > > --- a/common/filter
> > > +++ b/common/filter
> > > @@ -117,7 +117,7 @@ _filter_date()
> > > =20
> > >  # prints filtered output on stdout, values (use eval) on stderr
> > >  # Non XFS filesystems always return a 4k block size and a 256 byte i=
node.
> > > -_filter_mkfs()
> > > +_xfs_filter_mkfs()
> > >  {
> > >      case $FSTYP in
> > >      xfs)
> > > diff --git a/common/xfs b/common/xfs
> >=20
> > This renames the generic function to be "only for xfs" but it leaves th=
e
> > non-XFS bits.  Those bits are /really/ problematic (hardcoded
> > isize=3D256 and dbsize=3D4096?  Seriously??) and themselves were introd=
uced
> > in commit a4d5b247 ("xfstests: Make 204 work with different block and
> > inode sizes.") oh wow.
> >=20
> > I'm sorry that someone left this a mess, but let's try to make it easy
> > to clean up all the other filesystems, please.  Specifically, could you
> > please:
> >=20
> > 1. Hoist the XFS-specific code from _filter_mkfs into a new
> >    helper _xfs_filter_mkfs() in common/xfs?
>=20
> UGH.  I pressed <Send>, not <Save>.  Picking up from where I left off:
>=20
> 2. Make the generic _filter_mkfs function call the XFS-specific one,
> ala:
>=20
> _filter_mkfs()
> {
>     case $FSTYP in
>     xfs)
> 	_xfs_filter_mkfs "$@"
> 	;;
>     *)
> 	cat - >/dev/null
> 	perl -e 'print STDERR "dbsize=3D4096\nisize=3D256\n"'
> 	return ;;
>     esac
> }
>=20
> This way you don't have to make a gigantic treewide change, and we can
> start to make this function work properly for filesystems that have the
> ability to do an offline geometry dump (aka dumpe2fs for ext*).

Thank you for the comment. My thought was a rather negative one: I assumed =
that
_filter_mkfs would not support other filesystems than xfs. I will add a pat=
ch
in v2 series based on your suggestion above, expecting that _filter_mkfs wi=
ll
support other filesystems in the future.

--=20
Best Regards,
Shin'ichiro Kawasaki=