On Thu, Feb 18 2016, Qu Wenruo wrote: > + > +/* > + * Dedup storage backend > + * On disk is persist storage but overhead is large > + * In memory is fast but will lose all its hash on umount > + */ > +#define BTRFS_DEDUP_BACKEND_INMEMORY 0 > +#define BTRFS_DEDUP_BACKEND_ONDISK 1 > +#define BTRFS_DEDUP_BACKEND_LAST 2
Hi, This may seem petty, but I'm here to complain about the names. :-) Firstly, "2" is *not* the LAST backend. The LAST backed is clearly "ONDISK" with is "1:. "2" is the number of backends, or the count of them. So > +#define BTRFS_DEDUP_BACKEND_LAST 1 would be OK, as would > +#define BTRFS_DEDUP_BACKEND_COUNT 2 but what you have is wrong. The place where you use this define: + if (backend >= BTRFS_DEDUP_BACKEND_LAST) + return -EINVAL; is correct, but it looks wrong. It looks like it is saying that it is invalid to use the LAST backend! Secondly, you use "dup" as an abbreviation of "duplicate". The ioctl FIDEDUPERANGE and the tool duperemove both use "dupe". It would be nice if we could be consistent and all use the same abbreviation. Thanks, NeilBrown
signature.asc
Description: PGP signature
