Re: [PATCH v3 03/10] btrfs: create a helper function, check_fsid(), to verify the tempfsid

[Anand Jain <anand.jain@oracle.com>]

> function should do the require for everything it needs that may not be
> available.
> It's doing for the inspect-internal command, but it's missing a:
> 
> _require_btrfs_sysfs_fsid


Yes, it did. Actually, check_fsid() would need the following to
cover all the prerequisites.

  _require_btrfs_fs_sysfs
  _require_btrfs_fs_feature temp_fsid
  _require_btrfs_fs_feature metadata_uuid
  _require_btrfs_command inspect-internal dump-super


I already have v4 with what you just suggested, I am going to send it.


 > Instead this is being called for every test case that calls this new
 > helper function, when those requirements should be hidden from the
 > tests themselves.

However, I am a bit skeptical if we should move all prerequisites to
the helpers or only some major prerequisites.

Because returning _notrun() in the middle of the testcase is something
I am not sure is better than at the beginning of the testcase (I do not
have a specific example where it is not a good idea, though).

And, theoretically, figuring out if the test case would run/_notrun()
will be complicated.

Next, we shall end up checking the _require..() multiple times in
a test case, though one time is enough (the test cases 311, 312,
313 call check_fsid() two times).

Furthermore, it will inconsistent, as a lot of command wraps are
already missing such a requirement; I'm not sure if we shall ever
achieve consistency across fstests (For example: _cp_reflink()
missing _require_cp_reflink).

Lastly, if there are duplicating prerequisites across the helper
functions, then we call _require..() many more times (for example:
313 will call mkfs_clone() and check_fsid() two times, which
means we would verify the following three times in a testcase.

  _require_btrfs_fs_feature metadata_uuid
  _require_btrfs_command inspect-internal dump-super


So, how about prerequisites of the newer functions as comments
above the function to be copied into the test case?

Thanks, Anand