On 10/11/2016 07:44 PM, Mike Snitzer wrote:
On Tue, Oct 11 2016 at 11:44am -0400,
Heinz Mauelshagen <heinzm@xxxxxxxxxx> wrote:
On 10/11/2016 05:38 PM, Andy Whitcroft wrote:
On Tue, Oct 11, 2016 at 05:04:34PM +0200, Heinz Mauelshagen wrote:
Andy,
good catch.
We should rather check for V190 support only in case any
compat feature flags are actually set.
{
+ if (le32_to_cpu(sb->compat_features) &&
+ le32_to_cpu(sb->compat_features) != FEATURE_FLAG_SUPPORTS_V190)
{
rs->ti->error = "Unable to assemble array: Unknown flag(s)
in compatible feature flags";
return -EINVAL;
}
If the feature flags are single bit combinations then I believe the
below does check exactly that. Checking for no 1s outside of the
expected features, caring not for the value of the valid bits:
+ if (le32_to_cpu(sb->compat_features) & ~(FEATURE_FLAG_SUPPORTS_V190)) {
with the possibilty to or in additional feature bits as they are added.
Thanks,
I prefer this to be easier readable.
Readable or not, the code with the != is _not_ future-proof. Whereas
Andy's solution is. If/when a new compat feature comes along then
FEATURE_FLAG_SUPPORTS_V190 would be replaced to be a macro that ORs all
the new compat features together (e.g. FEATURE_FLAG_COMPAT). E.g. how
dm-thin-metadata.c:__check_incompat_features() does.
If we'll have to introduce more feature flags in the future (e.g. for
clustered raid1
support), this is going to be based on the test_bit() API for consistency
with any other flag processing we do in the target.
Heinz
We can go with the != code for now, since any future changes would
likely cause this test to be changed. Or we could fix it now _for
real_.
Mike
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
