Re: [PATCH 6/6] xfstests: Add mkfs input validation tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 21, 2016 at 1:54 AM, Dave Chinner <david@xxxxxxxxxxxxx> wrote:
>> >> I think you need to test on a 4k sector size disk. I use scsi_debug to
>> >> simulate physical 4k sector disk to reproduce this:
>> >>
>> >> [root@dhcp-66-86-11 xfsprogs-dev]# modprobe -r scsi_debug
>> >> [root@dhcp-66-86-11 xfsprogs-dev]# modprobe scsi_debug dev_size_mb=128 physblk_exp=3
>> >> [root@dhcp-66-86-11 xfsprogs-dev]# blockdev --getbsz --getpbsz --getss /dev/sdc
>> >> 4096
>> >> 4096
>> >> 512
>> >> [root@dhcp-66-86-11 xfsprogs-dev]# mkfs -t xfs -l version=1 -m crc=1 /dev/sdc
>
> So this is an invalid filesystem configuration. It should be
> detected as such during command line parsing and rejected before we
> get anywhere near checking topology constraints. In mkfs
> terms, it's a conflicting option set.
>
>> And the culprit is in mkfs, some forty lines before the crc & log version check:
>>
>> 2026 ⇥       } else if (lsectorsize > XFS_MIN_SECTORSIZE && !lsu && !lsunit) {
>> 2027 ⇥       ⇥       lsu = blocksize;
>> 2028 ⇥       ⇥       sb_feat.log_version = 2;
>> 2029 ⇥       }
>>
>> The possible solutions I can think of are:
>
> None of which really appeal because, IMO, they are trying to solve
> the wrong problem.
>
> The whole point of moving to table based command line option parsing
> is that we can encode these sorts of conflicts into the option
> table. The conflict resolution in the option table is currently not
> complete - it can only encode and detect conflicts within a
> suboption type, but not across suboption types (e.g. within -d
> suboptions, but not between -d and -l suboptions).
>
> This is simply because I never got as far as implementing this level
> of conflict encoding/resolution. In essence, the conflict array
> needs to define the sub option type, the suboption that is
> in conflict and the value that it conflicts against. Hence the
> conflicts table can then encode such things as "version 1 logs are
> invalid for CRC enabled filesystems" and vice versa.
>

Ok, in long term, the correct way is to extend the conflicts table.
But what in the meantime? Are we going to let it be now until it is
fixed by the enhanced table?
And regarding my question at the end of the mail, I interpret your
answer as "if the arguments are wrong, fail ASAP and don't try to fix
it."

Thanks,
Jan

-- 
Jan Tulak
jtulak@xxxxxxxxxx / jan@xxxxxxxx

_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs




[Index of Archives]     [Linux XFS Devel]     [Linux Filesystem Development]     [Filesystem Testing]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux