Correct Linux 4.X group_info fields detection

[szubersk]

Motivation and Context

Issues found while testing #13339.

Description

Check for struct group_info.nblocks field existence instead of
struct group_info.gid for Linux 4.9 API change.
Fixes compilation for Linux longterm branches.

How Has This Been Tested?

$ make

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[behlendorf]

How exactly is this check failing in the longterm branches? I ask, because normally we try and write the configure tests to detect the new interface (->gid in this case), and then fallback to the legacy interface. This tends to work fairly well since the legacy interface is unlikely to change, and because it allows us to keep the check when we can finally retire the legacy compatibility code.

[behlendorf]

Suggested change

	struct cred cr;
	struct cred cr __attribute__ ((unused));

This works, but we've tried to use the __attribute__ ((unused)) in the m4 config files instead of (void)cr.

Same change in ZFS_AC_KERNEL_SRC_GROUP_INFO_NBLOCKS

[szubersk]

How exactly is this check failing in the longterm branches? I ask, because normally we try and write the configure tests to detect the new interface (->gid in this case), and then fallback to the legacy interface. This tends to work fairly well since the legacy interface is unlikely to change, and because it allows us to keep the check when we can finally retire the legacy compatibility code.

Please take OpenZFS master and Linux 4.9.310, run

debian@vm:~/linux-4.9.310$ cat ../COMPILE_KERNEL 
make mrproper && make allnoconfig ARCH=x86_64 && echo -e 'CONFIG_MODULES=y\nCONFIG_CRYPTO=y\nCONFIG_BLOCK=y\nCONFIG_CRYPTO_DEFLATE=m\nCONFIG_ZLIB_DEFLATE=m\nCONFIG_ZLIB_INFLATE=m' >> .config && make olddefconfig ARCH=x86_64 && make -j10 -l5 bzImage modules ARCH=x86_64
...
debian@vm:~/zfs-portable$ git clean -dfxq ; ./autogen.sh && ./configure --with-config=kernel --with-linux=$HOME/linux-4.9.310 && make -j10 -l5
...
/home/debian/zfs-portable/module/spl/../os/linux/spl/spl-cred.c: In function 'crgetngroups':
/home/debian/zfs-portable/module/spl/../os/linux/spl/spl-cred.c:83:11: error: 'NGROUPS_PER_BLOCK' undeclared (first use in this function)
   83 |  if (rc > NGROUPS_PER_BLOCK) {
      |           ^~~~~~~~~~~~~~~~~
/home/debian/zfs-portable/module/spl/../os/linux/spl/spl-cred.c:83:11: note: each undeclared identifier is reported only once for each function it appears in
/home/debian/zfs-portable/module/spl/../os/linux/spl/spl-cred.c: In function 'crgetgroups':
/home/debian/zfs-portable/module/spl/../os/linux/spl/spl-cred.c:108:8: error: 'struct group_info' has no member named 'nblocks'
  108 |  if (gi->nblocks > 0)
      |        ^~
In file included from /home/debian/zfs-portable/module/spl/../os/linux/spl/spl-cred.c:26:
/home/debian/zfs-portable/module/spl/../os/linux/spl/spl-cred.c:109:27: error: 'struct group_info' has no member named 'blocks'
  109 |   gids = KGIDP_TO_SGIDP(gi->blocks[0]);
      |                           ^~
/home/debian/zfs-portable/include/os/linux/spl/sys/cred.h:46:30: note: in definition of macro 'KGIDP_TO_SGIDP'
   46 | #define KGIDP_TO_SGIDP(x) (&(x)->val)
      |                              ^

This might be due to the minimalistic nature of the kernel I compiled.

EDIT01
Ok, now I see I was wrong. Good catch, Brian!

debian@vm:~/zfs-portable/build/group_info_gid$ make modules -C /home/debian/linux-4.9.310  M=/home/debian/zfs-portable/build/group_info_gid
make: Entering directory '/home/debian/linux-4.9.310'
  CC [M]  /home/debian/zfs-portable/build/group_info_gid/group_info_gid.o
/home/debian/zfs-portable/build/group_info_gid/group_info_gid.c: In function 'main':
/home/debian/zfs-portable/build/group_info_gid/group_info_gid.c:72:27: error: implicit declaration of function 'groups_alloc' [-Werror=implicit-function-declaration]
   72 |   struct group_info *gi = groups_alloc(1);
      |                           ^~~~~~~~~~~~
/home/debian/zfs-portable/build/group_info_gid/group_info_gid.c:72:27: error: initialization of 'struct group_info *' from 'int' makes pointer from integer without a cast [-Werror=int-conversion]
cc1: all warnings being treated as errors
make[1]: *** [scripts/Makefile.build:314: /home/debian/zfs-portable/build/group_info_gid/group_info_gid.o] Error 1
make: *** [Makefile:1544: _module_/home/debian/zfs-portable/build/group_info_gid] Error 2
make: Leaving directory '/home/debian/linux-4.9.310'

EDIT02
Looks like without CONFIG_MULTIUSER Linux headers do not define a stub for groups_alloc() resulting in an compilation error.

#ifdef CONFIG_MULTIUSER
extern struct group_info *groups_alloc(int);
extern void groups_free(struct group_info *);

extern int in_group_p(kgid_t);
extern int in_egroup_p(kgid_t);
#else
static inline void groups_free(struct group_info *group_info)
{
}

static inline int in_group_p(kgid_t grp)
{
        return 1;
}
static inline int in_egroup_p(kgid_t grp)
{
        return 1;
}                                                                                                                                  
#endif

Enhancing the minimalistic config to the following form helps.

echo -e 'CONFIG_MODULES=y\nCONFIG_CRYPTO=y\nCONFIG_BLOCK=y\nCONFIG_MULTIUSER=y\nCONFIG_CRYPTO_DEFLATE=m\nCONFIG_ZLIB_DEFLATE=m\nCONFIG_ZLIB_INFLATE=m'

Thanks for helping catch the erorr. The (void)var; correction stemmed from Linux 3.X compilation which is not relevant anymore. I abandon this PR.

[behlendorf]

Ahh, that makes sense. Thanks for taking a second look. We could add some additional checks to ensure the legacy interface exists. You'd still see the failure without CONFIG_MULTIUSER defined, but at least you'd catch it at configure time. Though it may not be worth it for this corner case.