Re: [PATCH v2] fstests: btrfs/159 superblock corruption test case

Anand Jain
2018-04-09T05:22:10Z

>> +mount_opt_minus_args()
>> +{
>> + local nr
>> + local mnt_opt=""
>> +
>> + nr=`_scratch_mount_options | awk '{print NF}'`
>> + if [ $nr -eq 4 ]; then
>
> Seems this only works with "scratch_mount_options" returns something
> like:
>
> "-o opt dev mnt" or "-o opt1,opt2 dev mnt"
>
> but not
>
> "-oopt dev mnt" nor
> "-o opt1 -o opt2 dev mnt" nor if $SELINUX_MOUNT_OPTIONS not empty.
>
> Also if MOUNT_OPTIONS is "-oopt1 -oopt2", mount_opt_minus_args would
> return something like "-o -oopt2,device=<dev>", which are not valid
> mount options.

Oh. Yes. This function is getting complicated. I have dropped this
function in v3 which is kind of much better.

>> +# Test if the mount fails if the primary superblock csum is corrupted.
>> +check_primary()
>> +{
>> + local bytenr=65536
>> + echo -e "\\ncheck_primary\\n" | tee -a $seqres.full
>> +
>> + wipe
>> + _scratch_pool_mkfs "-mraid1 -draid1"
>> + _reload_fs_module "btrfs"
>> +
>> + #corrupt bytenr DEV_BAD
>> + od -j$bytenr -N1 -An -x $DEV_BAD |\
>> + dd status=none of=$DEV_BAD ibs=1 obs=1 seek=$bytenr count=1 conv=fsync
>> +
>> + #must fail
>> + _mount `mount_opt_minus_args device=$DEV_GOOD` $DEV_BAD $SCRATCH_MNT 2>&1 |\
>> + _filter_error_mount
>
> If I read the code correctly, the purpose of mount_opt_minus_args is to
> remove the device and mount point mount arguments and keep other options
> if there's any, then append "-o device=<dev>" as mount options.
>
> If so, I think you could use "_common_dev_mount_options", e.g.
>
> _mount `_common_dev_mount_options -o device=$DEV_GOOD` $DEV_BAD $SCRATCH_MNT ...
>
> But do mount options really matter in this test? Could we just mount
> with "-o device=<dev>"?

In fact for this test it doesn't matter, yes we could just use
"-o device=<dev>". But I thought its xfstests rule that it must be
used. Anyway in V3, I am changing the device itself on which
corrupted-SB would reside, so now we can let the default _scratch_mount
do its job as usual. Which means it shall use the config mount options.
V3 is in the ML for the review.

Thanks, Anand

> Thanks,
> Eryu