On 5/19/20 3:38 AM, Chaitanya Kulkarni wrote:
> On 5/18/20 11:39 PM, xiakaixu1987@xxxxxxxxx wrote:
>> From: Kaixu Xia <kaixuxia@xxxxxxxxxxx>
>>
>> There are two places that set the configured sector sizes in validate_sectorsize,
>> actually we can simplify them and combine into one if statement.
>> Is it me or patch description seems to be longer than what is in the
> tree ?
>> Signed-off-by: Kaixu Xia <kaixuxia@xxxxxxxxxxx>
>> ---
>> mkfs/xfs_mkfs.c | 14 ++++----------
>> 1 file changed, 4 insertions(+), 10 deletions(-)
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index 039b1dcc..e1904d57 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -1696,14 +1696,6 @@ validate_sectorsize(
>> int dry_run,
>> int force_overwrite)
>> {
>> - /* set configured sector sizes in preparation for checks */
>> - if (!cli->sectorsize) {
>> - cfg->sectorsize = dft->sectorsize;
>> - } else {
>> - cfg->sectorsize = cli->sectorsize;
>> - }
>> - cfg->sectorlog = libxfs_highbit32(cfg->sectorsize);
>> -
>
> If above logic is correct which I've not looked into it, then dft is
> not used in validate_sectorsize(), how about something like this on
> the top of this this patch (totally untested):-
Honestly if not set via commandline, and probing fails, we should fall
back to dft->sectorsize so that all the defaults are still set in one place,
i.e. the defaults structure mkfs_default_params.
-Eric
