I looked this morning to see who from user space uses what from lustre_idl.h.
--- lnet/utils/lst.c OBD_OCD_VERSION --- lustre/tests/multiop.c Use Macro lustre/include/lustre/lustre_idl.h:1612:9:LOV_PATTERN_RAID0 lustre/tests/multiop.c Use Macro lustre/include/lustre/lustre_idl.h:1618:9:LOV_PATTERN_F_RELEASED lustre/tests/multiop.c Use Macro lustre/include/lustre/lustre_idl.h:2460:9:FMODE_READ lustre/tests/multiop.c Use Macro lustre/include/lustre/lustre_idl.h:2461:9:FMODE_WRITE lustre/tests/multiop.c --- lustre/utils/lctl.c OBD_OCD_VERSION --- lustre/utils/lfs.c Use Enum lustre/include/lustre/lustre_idl.h:2750:6:lmv_hash_type lustre/utils/lfs.c:1548:15 Use Enum lustre/include/lustre/lustre_idl.h:2750:6:lmv_hash_type lustre/utils/lfs.c:1550:15 Use EnumConstant lustre/include/lustre/lustre_idl.h:2751:2:LMV_HASH_TYPE_ALL_CHARS lustre/utils/lfs.c:1550:15 Use EnumConstant lustre/include/lustre/lustre_idl.h:2752:2:LMV_HASH_TYPE_FNV_1A_64 lustre/utils/lfs.c:1548:15 Use Function lustre/include/lustre/lustre_idl.h:901:20:fid_is_sane lustre/utils/lfs.c:3174:7 Use Macro lustre/include/lustre/lustre_idl.h:1427:9:OBD_OCD_VERSION lustre/utils/lfs.c Use Macro lustre/include/lustre/lustre_idl.h:1612:9:LOV_PATTERN_RAID0 lustre/utils/lfs.c Use Macro lustre/include/lustre/lustre_idl.h:1618:9:LOV_PATTERN_F_RELEASED lustre/utils/lfs.c Use Macro lustre/include/lustre/lustre_idl.h:1899:9:OBD_OBJECT_EOF lustre/utils/lfs.c Use Macro lustre/include/lustre/lustre_idl.h:1978:9:toqb lustre/utils/lfs.c Use Macro lustre/include/lustre/lustre_idl.h:2006:9:Q_GETOINFO lustre/utils/lfs.c Use Macro lustre/include/lustre/lustre_idl.h:2007:9:Q_GETOQUOTA lustre/utils/lfs.c Use Macro lustre/include/lustre/lustre_idl.h:2756:9:LMV_HASH_NAME_ALL_CHARS lustre/utils/lfs.c Use Macro lustre/include/lustre/lustre_idl.h:2757:9:LMV_HASH_NAME_FNV_1A_64 lustre/utils/lfs.c --- lustre/utils/l_getidentity.c Use EnumConstant lustre/include/lustre/lustre_idl.h:2384:9:CFS_SETUID_PERM lustre/utils/l_getidentity.c:201:21 Use EnumConstant lustre/include/lustre/lustre_idl.h:2384:9:CFS_SETUID_PERM lustre/utils/l_getidentity.c:210:23 Use EnumConstant lustre/include/lustre/lustre_idl.h:2385:9:CFS_SETGID_PERM lustre/utils/l_getidentity.c:202:21 Use EnumConstant lustre/include/lustre/lustre_idl.h:2385:9:CFS_SETGID_PERM lustre/utils/l_getidentity.c:211:23 Use EnumConstant lustre/include/lustre/lustre_idl.h:2386:9:CFS_SETGRP_PERM lustre/utils/l_getidentity.c:203:21 Use EnumConstant lustre/include/lustre/lustre_idl.h:2386:9:CFS_SETGRP_PERM lustre/utils/l_getidentity.c:212:23 Use EnumConstant lustre/include/lustre/lustre_idl.h:2387:9:CFS_RMTACL_PERM lustre/utils/l_getidentity.c:204:21 Use EnumConstant lustre/include/lustre/lustre_idl.h:2387:9:CFS_RMTACL_PERM lustre/utils/l_getidentity.c:213:23 Use EnumConstant lustre/include/lustre/lustre_idl.h:2388:9:CFS_RMTOWN_PERM lustre/utils/l_getidentity.c:205:21 Use EnumConstant lustre/include/lustre/lustre_idl.h:2388:9:CFS_RMTOWN_PERM lustre/utils/l_getidentity.c:214:23 --- lustre/utils/lhsmtool_posix.c Use Function lustre/include/lustre/lustre_idl.h:574:20:fid_is_igif lustre/utils/lhsmtool_posix.c:1533:29 Use Function lustre/include/lustre/lustre_idl.h:604:20:fid_is_norm lustre/utils/lhsmtool_posix.c:1533:9 Use Macro lustre/include/lustre/lustre_idl.h:1721:9:XATTR_TRUSTED_PREFIX lustre/utils/lhsmtool_posix.c --- lustre/utils/liblustreapi.c Use Enum lustre/include/lustre/lustre_idl.h:437:6:fid_seq lustre/utils/liblustreapi.c:2220:44 Use Enum lustre/include/lustre/lustre_idl.h:437:6:fid_seq lustre/utils/liblustreapi.c:3313:13 Use EnumConstant lustre/include/lustre/lustre_idl.h:467:2:FID_SEQ_LOV_DEFAULT lustre/utils/liblustreapi.c:2220:44 Use EnumConstant lustre/include/lustre/lustre_idl.h:467:2:FID_SEQ_LOV_DEFAULT lustre/utils/liblustreapi.c:3313:13 Use Field lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path:gf_fid lustre/utils/liblustreapi.c:4253:13 Use Field lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path:gf_linkno lustre/utils/liblustreapi.c:4255:13 Use Field lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path:gf_linkno lustre/utils/liblustreapi.c:4266:31 Use Field lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path:gf_path lustre/utils/liblustreapi.c:4264:33 Use Field lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path:gf_pathlen lustre/utils/liblustreapi.c:4256:13 Use Field lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path:gf_pathlen lustre/utils/liblustreapi.c:4264:46 Use Field lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path:gf_recno lustre/utils/liblustreapi.c:4254:13 Use Field lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path:gf_recno lustre/utils/liblustreapi.c:4265:30 Use Function lustre/include/lustre/lustre_idl.h:1673:20:lmm_oi_set_seq lustre/utils/liblustreapi.c:2221:3 Use Function lustre/include/lustre/lustre_idl.h:1683:21:lmm_oi_id lustre/utils/liblustreapi.c:2235:9 Use Function lustre/include/lustre/lustre_idl.h:1688:21:lmm_oi_seq lustre/utils/liblustreapi.c:2220:16 Use Function lustre/include/lustre/lustre_idl.h:1688:21:lmm_oi_seq lustre/utils/liblustreapi.c:2233:9 Use Function lustre/include/lustre/lustre_idl.h:419:20:fid_zero lustre/utils/liblustreapi.c:4123:3 Use Function lustre/include/lustre/lustre_idl.h:419:20:fid_zero lustre/utils/liblustreapi.c:4124:3 Use Function lustre/include/lustre/lustre_idl.h:490:20:fid_seq_is_mdt0 lustre/utils/liblustreapi.c:2372:7 Use Function lustre/include/lustre/lustre_idl.h:521:20:fid_seq_is_rsvd lustre/utils/liblustreapi.c:2371:7 Use Function lustre/include/lustre/lustre_idl.h:638:23:ostid_seq lustre/utils/liblustreapi.c:2366:40 Use Function lustre/include/lustre/lustre_idl.h:653:22:ostid_id lustre/utils/liblustreapi.c:2365:41 Use Function lustre/include/lustre/lustre_idl.h:668:20:ostid_set_seq lustre/utils/liblustreapi.c:3312:6 Use Function lustre/include/lustre/lustre_idl.h:880:20:fid_le_to_cpu lustre/utils/liblustreapi.c:4286:2 Use Function lustre/include/lustre/lustre_idl.h:901:20:fid_is_sane lustre/utils/liblustreapi.c:4243:14 Use Macro lustre/include/lustre/lustre_idl.h:1618:9:LOV_PATTERN_F_RELEASED lustre/utils/liblustreapi.c Use Macro lustre/include/lustre/lustre_idl.h:1726:9:XATTR_NAME_LMA lustre/utils/liblustreapi.c Use Macro lustre/include/lustre/lustre_idl.h:2745:9:LMV_MAGIC_V1 lustre/utils/liblustreapi.c Use Macro lustre/include/lustre/lustre_idl.h:2746:9:LMV_USER_MAGIC lustre/utils/liblustreapi.c Use Macro lustre/include/lustre/lustre_idl.h:3109:9:MTI_NAME_MAXLEN lustre/utils/liblustreapi.c Use Macro lustre/include/lustre/lustre_idl.h:3343:9:CHANGELOG_USER_PREFIX lustre/utils/liblustreapi.c Use Record lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path lustre/utils/liblustreapi.c:4250:21 Use Record lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path lustre/utils/liblustreapi.c:4253:13 Use Record lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path lustre/utils/liblustreapi.c:4254:13 Use Record lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path lustre/utils/liblustreapi.c:4255:13 Use Record lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path lustre/utils/liblustreapi.c:4256:13 Use Record lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path lustre/utils/liblustreapi.c:4264:33 Use Record lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path lustre/utils/liblustreapi.c:4264:46 Use Record lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path lustre/utils/liblustreapi.c:4265:30 Use Record lustre/include/lustre/lustre_idl.h:3866:8:getinfo_fid2path lustre/utils/liblustreapi.c:4266:31 --- lustre/utils/liblustreapi_hsm.c Use Enum lustre/include/lustre/lustre_idl.h:1858:6:hss_valid lustre/utils/liblustreapi_hsm.c:1352:18 Use Enum lustre/include/lustre/lustre_idl.h:1858:6:hss_valid lustre/utils/liblustreapi_hsm.c:1352:30 Use Enum lustre/include/lustre/lustre_idl.h:1858:6:hss_valid lustre/utils/liblustreapi_hsm.c:1358:20 Use EnumConstant lustre/include/lustre/lustre_idl.h:1859:2:HSS_SETMASK lustre/utils/liblustreapi_hsm.c:1352:18 Use EnumConstant lustre/include/lustre/lustre_idl.h:1860:2:HSS_CLEARMASK lustre/utils/liblustreapi_hsm.c:1352:30 Use EnumConstant lustre/include/lustre/lustre_idl.h:1861:2:HSS_ARCHIVE_ID lustre/utils/liblustreapi_hsm.c:1358:20 Use Field lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set:hss_archive_id lustre/utils/liblustreapi_hsm.c:1359:7 Use Field lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set:hss_clearmask lustre/utils/liblustreapi_hsm.c:1354:6 Use Field lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set:hss_setmask lustre/utils/liblustreapi_hsm.c:1353:6 Use Field lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set:hss_valid lustre/utils/liblustreapi_hsm.c:1352:6 Use Field lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set:hss_valid lustre/utils/liblustreapi_hsm.c:1358:7 Use Macro lustre/include/lustre/lustre_idl.h:1612:9:LOV_PATTERN_RAID0 lustre/utils/liblustreapi_hsm.c Use Macro lustre/include/lustre/lustre_idl.h:1618:9:LOV_PATTERN_F_RELEASED lustre/utils/liblustreapi_hsm.c Use Record lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set lustre/utils/liblustreapi_hsm.c:1349:24 Use Record lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set lustre/utils/liblustreapi_hsm.c:1352:6 Use Record lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set lustre/utils/liblustreapi_hsm.c:1353:6 Use Record lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set lustre/utils/liblustreapi_hsm.c:1354:6 Use Record lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set lustre/utils/liblustreapi_hsm.c:1358:7 Use Record lustre/include/lustre/lustre_idl.h:1864:8:hsm_state_set lustre/utils/liblustreapi_hsm.c:1359:7 --- lustre/utils/llog_reader.c Use EnumConstant lustre/include/lustre/lustre_idl.h:3217:2:LLOG_PAD_MAGIC lustre/utils/llog_reader.c:517:8 Use EnumConstant lustre/include/lustre/lustre_idl.h:3227:2:OBD_CFG_REC lustre/utils/llog_reader.c:170:63 Use EnumConstant lustre/include/lustre/lustre_idl.h:3227:2:OBD_CFG_REC lustre/utils/llog_reader.c:512:8 Use EnumConstant lustre/include/lustre/lustre_idl.h:3233:2:HSM_AGENT_REC lustre/utils/llog_reader.c:523:8 Use EnumConstant lustre/include/lustre/lustre_idl.h:3235:2:LLOG_LOGID_MAGIC lustre/utils/llog_reader.c:520:8 Use Field lustre/include/lustre/lustre_idl.h:2898:8:lov_desc:ld_default_stripe_count lustre/utils/llog_reader.c:262:48 Use Field lustre/include/lustre/lustre_idl.h:2898:8:lov_desc:ld_default_stripe_offset lustre/utils/llog_reader.c:264:49 Use Field lustre/include/lustre/lustre_idl.h:2898:8:lov_desc:ld_default_stripe_size lustre/utils/llog_reader.c:263:47 Use Field lustre/include/lustre/lustre_idl.h:2898:8:lov_desc:ld_pattern lustre/utils/llog_reader.c:265:45 Use Field lustre/include/lustre/lustre_idl.h:2898:8:lov_desc:ld_uuid lustre/utils/llog_reader.c:261:54 ### And 140 more lines (llog_logid, llog_rec_hdr, llog_agent_req_rec) ### WONTFIX (for now, maybe never). --- lustre/utils/ll_recover_lost_found_objs.c Use Function lustre/include/lustre/lustre_idl.h:490:20:fid_seq_is_mdt0 lustre/utils/ll_recover_lost_found_objs.c:215:7 Use Function lustre/include/lustre/lustre_idl.h:490:20:fid_seq_is_mdt0 lustre/utils/ll_recover_lost_found_objs.c:362:8 Use Function lustre/include/lustre/lustre_idl.h:490:20:fid_seq_is_mdt0 lustre/utils/ll_recover_lost_found_objs.c:373:8 Use Function lustre/include/lustre/lustre_idl.h:521:20:fid_seq_is_rsvd lustre/utils/ll_recover_lost_found_objs.c:214:21 Use Function lustre/include/lustre/lustre_idl.h:521:20:fid_seq_is_rsvd lustre/utils/ll_recover_lost_found_objs.c:361:22 Use Function lustre/include/lustre/lustre_idl.h:521:20:fid_seq_is_rsvd lustre/utils/ll_recover_lost_found_objs.c:372:22 Use Function lustre/include/lustre/lustre_idl.h:584:20:fid_seq_is_idif lustre/utils/ll_recover_lost_found_objs.c:216:7 Use Function lustre/include/lustre/lustre_idl.h:584:20:fid_seq_is_idif lustre/utils/ll_recover_lost_found_objs.c:363:4 Use Function lustre/include/lustre/lustre_idl.h:584:20:fid_seq_is_idif lustre/utils/ll_recover_lost_found_objs.c:374:8 --- lustre/utils/loadgen.c Use Enum lustre/include/lustre/lustre_idl.h:1546:6:obdo_flags lustre/utils/loadgen.c:567:27 Use EnumConstant lustre/include/lustre/lustre_idl.h:1553:9:OBD_FL_DEBUG_CHECK lustre/utils/loadgen.c:567:27 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_flags lustre/utils/loadgen.c:567:17 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_gid lustre/utils/loadgen.c:486:17 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_mode lustre/utils/loadgen.c:482:17 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_mode lustre/utils/loadgen.c:523:24 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_mode lustre/utils/loadgen.c:564:17 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_oi lustre/utils/loadgen.c:483:37 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_oi lustre/utils/loadgen.c:484:31 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_oi lustre/utils/loadgen.c:503:42 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_oi lustre/utils/loadgen.c:522:31 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_oi lustre/utils/loadgen.c:562:37 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_oi lustre/utils/loadgen.c:563:31 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_uid lustre/utils/loadgen.c:485:17 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_valid lustre/utils/loadgen.c:487:17 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_valid lustre/utils/loadgen.c:497:30 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_valid lustre/utils/loadgen.c:499:51 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_valid lustre/utils/loadgen.c:524:24 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_valid lustre/utils/loadgen.c:565:17 Use Field lustre/include/lustre/lustre_idl.h:3487:8:obdo:o_valid lustre/utils/loadgen.c:574:32 Use Function lustre/include/lustre/lustre_idl.h:653:22:ostid_id lustre/utils/loadgen.c:503:17 Use Function lustre/include/lustre/lustre_idl.h:687:20:ostid_set_seq_echo lustre/utils/loadgen.c:483:2 Use Function lustre/include/lustre/lustre_idl.h:687:20:ostid_set_seq_echo lustre/utils/loadgen.c:562:2 Use Function lustre/include/lustre/lustre_idl.h:701:20:ostid_set_id lustre/utils/loadgen.c:484:2 Use Function lustre/include/lustre/lustre_idl.h:701:20:ostid_set_id lustre/utils/loadgen.c:522:2 Use Function lustre/include/lustre/lustre_idl.h:701:20:ostid_set_id lustre/utils/loadgen.c:563:2 Use Macro lustre/include/lustre/lustre_idl.h:1784:9:OBD_MD_FLID lustre/utils/loadgen.c Use Macro lustre/include/lustre/lustre_idl.h:1789:9:OBD_MD_FLBLOCKS lustre/utils/loadgen.c Use Macro lustre/include/lustre/lustre_idl.h:1791:9:OBD_MD_FLMODE lustre/utils/loadgen.c Use Macro lustre/include/lustre/lustre_idl.h:1792:9:OBD_MD_FLTYPE lustre/utils/loadgen.c Use Macro lustre/include/lustre/lustre_idl.h:1793:9:OBD_MD_FLUID lustre/utils/loadgen.c Use Macro lustre/include/lustre/lustre_idl.h:1794:9:OBD_MD_FLGID lustre/utils/loadgen.c Use Macro lustre/include/lustre/lustre_idl.h:1795:9:OBD_MD_FLFLAGS lustre/utils/loadgen.c Use Macro lustre/include/lustre/lustre_idl.h:1807:9:OBD_MD_FLGROUP lustre/utils/loadgen.c Use Macro lustre/include/lustre/lustre_idl.h:1811:9:OBD_MD_FLGRANT lustre/utils/loadgen.c Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:482:17 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:483:37 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:484:31 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:485:17 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:486:17 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:487:17 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:497:30 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:499:51 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:503:42 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:522:31 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:523:24 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:524:24 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:562:37 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:563:31 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:564:17 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:565:17 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:567:17 Use Record lustre/include/lustre/lustre_idl.h:3487:8:obdo lustre/utils/loadgen.c:574:32 ### Does anybody use this? --- lustre/utils/lustre_cfg.c Use Macro lustre/include/lustre/lustre_idl.h:1427:9:OBD_OCD_VERSION lustre/utils/lustre_cfg.c --- lustre/utils/lustre_rsync.c Use Function lustre/include/lustre/lustre_idl.h:901:20:fid_is_sane lustre/utils/lustre_rsync.c:1126:6 --- lustre/utils/mount_lustre.c Use Macro lustre/include/lustre/lustre_idl.h:1427:9:OBD_OCD_VERSION lustre/utils/mount_lustre.c --- lustre/utils/obd.c ### 940 lines (obdo*, md_op_flags, OBD_FL*, LCK_*, odc_connect_data, lmv_desc, lov_desc, ...) ### WONTFIX. --- lustre/utils/wiretest.c ### 3080 lines (looks like the phone book) ### WONTFIX (obvs).
Also the following macros are defined in both files:
LMV_MAGIC_V1 LMV_USER_MAGIC LOV_PATTERN_FIRST LOV_PATTERN_RAID0 LOV_PATTERN_RAID1 XATTR_LUSTRE_PREFIX
Based on lots of different and (often contradictory) feedback I would like to try the following:
- Unpackage lustre_idl.h, libiam.h and lustre_lfsck_user.h.
- Add a sanity check that what ever lands in /usr/include/lustre can be compiled without warning. (Except liblustreapi.h which is deprecated.)
- Incrementally move towards implementing the following:
- If a structure is passed (via ioctl() or xattr) from kernel space to user space then it must be defined in lustre_user.h.
- Otherwise if it goes on the wire then it must be defined in lustre_idl.h.
- Otherwise it must go somewhere else.
- Trivial static inline accessors and macros like DFID/PFID can and should accompany the definitions of the structures the access. No CERRORs. No snprintf(). No memcpy(). No str*cpy(). DFID and PFID must be written without using LPX64 or anything else that depends on config.h for correctness.
- Random crap headers like <sys/quota.h> may not be pulled in by either of lustre_idl.h or lustre_user.h.
This is probably the best anyone can do in the short term and I am probably the only person who is willing to do this right now. It needs to be incremental and it won't always be pretty. For example I may have to add libcfs.h to lustre_idl.h initially. Note that's actually making things better than they are now since it's actually correct from the include what you use point of view. It's not the end goal but it makes the header graph more robust while small things happen elsewhere.
In short if I'm not given some breathing room on the patches that I submit then I'll probably just go and do something else.
Please see http://review.whamcloud.com/#/c/10215.
I really appreciate you working on this, John!
I think it going to help that you have documented your approach here for us to debate, and you can point to it in the future when folks have questions. I think that is going to help during reviews if we already agree with the design approach.
I like the this approach.
I have a minor quibble about the DFID/PFID style macros in the packaged headers. That just seems like too much of an internal lustrism to expose to user space. But like I said, it is just a quibble, and I won't hold patches up on that since it DFID/PFID is already in lustre_user.h.
lustre_rsync is currently an example of how to use the changelog API, and it is using DFID/PFID and fid_is_sane() and perhaps more (not prepared to read any more of it so soon after breakfast).
Actually, lustre_rsync.c is a pretty perfect example of why what we really want in an API are string-to-fid and fid-to-string functions rather than DFID/PFID:
sprintf(info->tfid, DFID, PFID(&rec->cr_tfid));
sprintf(info->pfid, DFID, PFID(&rec->cr_pfid));
Here DFID/PFID are not used in the inline fashion for which they were intended. A fid-to-string function would have been more appropriate here and resulted in much more readable (and thus maintainable) code.
It is not at all clear to me why the user space code would be using fid_is_sane.
In this case it is using fid_is_sane() to determine if the changelog record contains additional fids. I'm not following your comment about inline use of DFID though.
DFID/PFID are kind of an overly clever hack rather than good programming practice. In the kernel that sort of thing is more common, and I can overlook it. In user space, it is really not acceptable.
They were basically designed to be used like this:
printf("Fid of file A is "DFID" .\n", PFID(&fidA));
Now lets consider the readability of that line from the viewpoint of someone who doesn't know Lustre. First of all, string concatenation is not something seen very often in user space C code, so that might throw some folks off. Next, they need to stop reading the current line of code and go read the definitions of DFID and PFID, because those names aren't particularly descriptive, and this use of macros is really unusual. And of course there isn't even a comment to help the newbie understand the macros. So now we've broken their train of thought while they go off and figure out this unusual use of macros. So they figure it out and go "Ok, gee, that is clever...wait what was I doing?".
The lustre_rsync.c is usage is the degenerate case where there is no string concatenation at all. I'll readily admit that the macros are reasonably clever and powerful. But even the authors of lustre_rsync.c did not seem to really understand the power. Instead they use the macros to perform conversions to intermediate string forms. Later they then feed the intermediate string form to printf-like functions through the %s converstion specifier. That intermediate string form is completely unnecessary. They could have simply used the DFID/PFID macros in the printf-like functions. But the authors didn't seem to grasp that.
Consider this alternative to the above code:
printf("Fid of file A is %s.\n", fid_to_string(fidA));
No string concatenation, no macros that need to be looked up, nothing at all out of the ordinary. Just simple, understandable code. Granted, fid_to_string is not reentrant as written, and that is one issue that the macros solve. But the reentrance issue is one that is pretty well understood in the C world, and not specific to Lustre.
In this case it is using fid_is_sane() to determine if the changelog record contains additional fids.
That just seems wrong. There should be a more obvious way to delineate the end of the changelog record.
Right, I see that DFID is often used inline that way, but surely it is also intended to be used standalone because that is the same format used for open by fid and also the format needed for llapi_fid2path().
This brings me back to my preference to see a well defined userspace interface for Lustre that can be used directly, and to stop relying on "special" libraries to cover up the warts. Until then, I'd rather use a macro like DFID.
llapi_fid2path uses the other macros for the scanning form, SFID/RFID, not DFID/PFID. 
Exactly, which means it assumes that the fidstr was formatted using DFID.
And if I want to use any other parser on the face of the planet, what then?
well, not the multithreaded ones, right? 
Kidding aside - I think we've given John plenty to think about already, and I suggest we see what solutions he comes up with next.
So now we have:
- http://review.whamcloud.com/#/c/10215 to unpackage several headers
- http://review.whamcloud.com/#/c/10273 to make packaged headers not depend on config.h (rough draft)
John, I have a suggestion for patch 10273.
The fixed-size data types:
- int[8,16,32,64]_t
- uint[8,16,32,64]_t
were standardized in user space in C99 and defined in the header stdint.h. Granted, the kernel is still largly C89, and doesn't implement the full standard.
The kernel probably didn't have those same defines back when we started using the double-underscore names, but it looks to me like the kernel made a nod towards compatibility a while back by defining those same eight fixed-sized types in include/linux/types.h.
So perhaps instead of creating the new lu_* types, we could just use the more well known C99 names in both user- and kernel-space. For a header that needs to be included both places, the include snippet would be:
#ifdef __KERNEL__
#include <linux/types.h>
#else
#include <stdint.h>
#endif
Yes, I've seen the typedefs for the stdint.h types in types.h. However their use is generally discouraged in kernel code. There are also a few issues I have found with them:
- We don't control them (whether they come from linux/types.h or from stdint.h).
- We should try to avoid including them through our headers if at all possible.
- In the kernel uint64_t is defined to be __u64 which is usually unsigned long long.
- In stdint.h on x86_64 uint64_t is defined to be unsigned long.
- The kernel does not define the PRIx64 type macros.
I would like it if our library header could be written using C primitive types, stddef.h, and sys/types.h. But this will take some time. In the mean time, using names that we control, without depending on config.h, and without bringing in the __uXX names (see LU-5018) seems like a good first step.
It would definitely be appropriate for libraries that sit on top of the kernel interface to use <stdint.h> types in their higher level API. I don't think it's currently the right thing to do in the the interface between kernel and userspace.
I believe the normal thing to do (again, see fiemap) is to use linux/types.h (the __* types) for data being exchanged between user and kernel space. Why not just use that instead of posix-types.h or inventing new ones?
Actually in the patch we /do/ use those type to go between user and kernel space. We just don't want to package header that depend on or export those types (again LU-5018). Moreover you'll recall that all "identifiers that begin with an underscore and either an uppercase letter or another underscore are always reserved for any use" by the C standard library.
We don't control them (whether they come from linux/types.h or from stdint.h).
I think that is a good thing. This is a really basic, well-known data type of explicitly fixed size. We'll probably get them wrong somewhere on some architecture if we do them ourselves. We have gotten very similar things wrong in the past.
We should try to avoid including them through our headers if at all possible.
I am not sure that I understand the rationale for this assertion. Maybe a longer explanation is needed.
In the kernel uint64_t is defined to be __u64 which is usually unsigned long long.
I am not sure that I understand your point. That is the type that winds up being unsigned and 64 bits in the kernel for that particular architecture. Someone else figured that out for us so we don't have to. I see that as a good thing.
In stdint.h on x86_64 uint64_t is defined to be unsigned long.
I would use the same argument as above. That is good thing. They figured out which type was needed to deliver an unsigned 64 bit storage space. We will be forced to do exactly the same thing with our definitions. If we don't we will wind up with the wrong storage size in some places on some architectures.
If we tried to use unsigned long long in user space when unsigned long was the correct size, we could wind up using a 128 bit field where we wanted a 64 bit field. We are using fixed size data types for a reason when we define those structures. They need to be a particular fixed size. It is far better to use the work that the Linux and C language communities have done to define those values. They are much less likely to make a mistake then we are since their developer and user bases are so much larger than ours.
The kernel does not define the PRIx64 type macros.
That is certainly true. We can't expect everything from stdint.h to be available in the kernel. I didn't mean to imply that they would be. Your point is a good one, and I think we can address that by use of a good code comment. Something like this perhaps:
/* Note that many of definitions in stdint.h definitions are _not_ available in the kernel. * It is safe to use these eight types: * int8_t, int16_t, int32_t, int64_t, uint8_t, uint16_t, uint32_t, uint64_t */
without depending on config.h
By the way, I entirely agree with that point. User space should not depend on Lustre's config.h. I squashed some attempts to add further dependencies on that in the past year or two.
No please don't make my eyes bleed with lu_* cfs_* types. Chris is right in that the kernel types were developed before the C99 spec. It not true that C99 types are discouraged. In fact I quote from Documentation/CodingStyle
----------------------------------------------------------------------------------------------------------------------------
(d) New types which are identical to standard C99 types, in certain
exceptional circumstances.
Although it would only take a short amount of time for the eyes and
brain to become accustomed to the standard types like 'uint32_t',
some people object to their use anyway.
Therefore, the Linux-specific 'u8/u16/u32/u64' types and their
signed equivalents which are identical to standard types are
permitted – although they are not mandatory in new code of your
own.
When editing existing code which already uses one or the other set
of types, you should conform to the existing choices in that code.
------------------------------------------------------------------------------------------------------------------
Any non Lustre code I submit to the kernel always uses the C99 types. As for C99 types being broken that I really doubt. Their are millions of applications that depend on glibc getting it right. The last point to make is
we don't need to use linux types for kernel to userland interfaces as well. It it perfectly sane to use C99 types.
In fact to see a beautiful example of this take a look at
linux-source-tree/include/uapi/linux/fuse.h
Not a single __uXX crap thing there. Then take a look fuse_do_ioctl in linux-source-tree/fs/fuse/file.c. Again no __uXX crap. Lastly any code pushed upstream with special types will be immediately rejected.
Usually I'm pretty non bias on most things but this is not one of them.
I tried to use uint64_t et al in the packaged headers. It's just too subtle and too broken. When userspace includes linux/types.h it doesn't get uint64_t. So userspace would need to include stdint.h. But then uint64_t is defined using an inequivalent primitive type.
t:~# uname -r 2.6.32-431.5.1.el6.lustre.x86_64 t:~# cat /etc/redhat-release CentOS release 6.4 (Final) t:~# cat test-linux.c #include <linux/types.h> uint64_t u; t:~# gcc -Wall -c test-linux.c test-linux.c:3: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘u’ t:~# cat test-stdint.c #include <stdint.h> uint64_t u; t:~# gcc -Wall -c test-stdint.c -g t:~# gdb test-stdint.o GNU gdb (GDB) Red Hat Enterprise Linux (7.2-60.el6) ... Reading symbols from /root/test-stdint.o...done. (gdb) ptype u type = long unsigned int t:~# gdb lustre-release/lustre/llite/lustre.ko GNU gdb (GDB) Red Hat Enterprise Linux (7.2-60.el6) ... Reading symbols from /root/lustre-release/lustre/llite/lustre.ko...done. (gdb) ptype uint64_t type = long long unsigned int
But then uint64_t is defined using an inequivalent primitive type.
Yes, that is true. They are supposed to be different. The same primitive type is not necessarily the same size on the same platform in both kernel and user space. You cannot assume that they are, unfortunately. Which is why you want to let the kernel header define the value in the kernel, and let the user space glibc header define the value in user space. Then you wind up with the correctly sized field wherever you are.
I do understand your point about PRIx64 type macros, and perhaps glossed over it a bit too easily in my previously reply. But I believe we should deal with the issue of displaying the value separately from this decision. After all, we already have to do a whole bunch of work to create macros for printk in the kernel; what I am proposing would be no worse. On the other hand your lu_* suggestion then exposes all of that extra work to user space.
With my suggestion applications that are fully in user space get to use the easy PRI* macros, or any other method of their choosing. The in-kernel code that wants to display the values is no worse off than it is today (a whole bunch of painful to maintain, but necessary, macros).
Please see http://review.whamcloud.com/#/c/10280/ for what happens when we use uin32_t in the headers. The issue now is that if __KERNEL__ is defined then __u64 and uint64_t are the same type (unsigned long long). If __KERNEL__ is not defined then one is unsigned long and the other is unsigned long long. Thus we have a huge flag day where all of user space needs to be rewritten to not use __[us]XX. For sanity sake this also means not using LPX64. Whatever changes we make we still need to stay friends with GCC.
Regarding the fuse headers: yes they look very nice. The difference I see is that they probably share very little code between user space and kernel space. We share a lot. It's not great that we do, but we do. Please remember that there are limits to how much change I can convince the other developers to accept in a short time.
I'm not necessarily keen on introducing new types to userspace instead of __u32 and friends. Other packages that interface with the kernel (e.g. e2fsprogs) also need to use __u* types for ioctls. See http://review.whamcloud.com/10258 for a relatively simple start to this. It doesn't do as much cleanup as John's patch.
> I do understand your point about PRIx64 type macros, and perhaps glossed over it a bit too easily in my previously reply. But I believe we should deal with the issue of displaying the value separately from this decision. After all, we already have to do a whole bunch of work to create macros for printk in the kernel; what I am proposing would be no worse. On the other hand your lu_* suggestion then exposes all of that extra work to user space.
It's already done and it's less work than before. See http://review.whamcloud.com/#/c/10273.
By introducing "new types to userspace" which types do you mean?
- the __uXX from linux/types.h
- our __uXX from posix-types.h when linux/types.h is not available
- the uintXX_t from stdint.h?
- the lu_uintXX_t from libcfs/types.h?
- the obd_xxx from lustre_user.h
- none of the above
- some of the above
- all of the above
I was primary referring to 4, lu_uintXX_t. I think that takes us in the wrong direction.
I find __uXX types a little less bothersome. But if anything, it sees like we should use the uXX and sXX forms rather than the forms that are preceded with underscores when we wish to share with userspace.
Overall I am still thinking uintXX_t is probably the best way to go.
Actually, the reverse is true - the __u* versions are for interface usage, and u* is for kernel internal use. Please don't go the opposite way from the kernel.
Seems like an odd choice. But Linux is just odd that way sometimes, I guess.
I guess I'm fine with use of the __u* as long as it is for things that interface directly with the kernel.
Looking at a FC13 test system I have, <linux/types.h> is part of kernel-headers*.rpm, which is required by glibc-headers, which is required by glibc-headers, which is required by glibc-devel, which is required by gcc, so it does indeed seem that <linux/types.h> is going to be installed on any system that is compiling something on Linux.
I think it makes sense to go back to http://review.whamcloud.com/#/c/10258/1/libcfs/include/libcfs/posix/posix-types.h which includes <linux/types.h> on any Linux system.
> Looking at a FC13 test system I have, <linux/types.h> is part of kernel-headers*.rpm, which is required by glibc-headers, which is required by glibc-headers, which is required by glibc-devel, which is required by gcc, so it does indeed seem that <linux/types.h> is going to be installed on any system that is compiling something on Linux.
This is a tenuous dependency. None of the headers in /usr/include/, /usr/include/bits/ or /usr/include/sys/ include linux/types.h or use any of the __uXX typedefs. sys/kd.h even defines _LINUX_TYPES_H so that including linux/kd.h will not pull in linux/types.h.
> I think it makes sense to go back to http://review.whamcloud.com/#/c/10258/1/libcfs/include/libcfs/posix/posix-types.h which includes <linux/types.h> on any Linux system.
I agree that in the short term this is the best fix.
I tried against the latest master build #2072, no warnings.
It seems sanity.sh 400a and 400b have done the tests already
[root@client-18 ~]# cat 5011_example.c
#include <lustre/lustreapi.h>
#include <lustre/lustre_user.h>
#include <lustre/ll_fiemap.h>
main() {}
[root@client-18 ~]# gcc 5011_example.c -llustreapi
[root@client-18 ~]#
Thanks Sarah. This ticket will remain open as there are other subtasks still open but it sounds like the most critical tasks that were considered in scope for 2.6 are now complete so I am dropping that FixVersion and lowering the priority
Wait, wait, wait. Why did you test lustreapi.h, lustre_user.h, and ll_fiemap.h when this ticket is very explicitly about lustre_idl.h?
Hi Christopher,
I didn't test lustre_idl.h because it has been removed
http://review.whamcloud.com/#/c/10215
[root@client-18 lustre]# ls
liblustreapi.h ll_fiemap.h lustreapi.h lustre_user.h
[root@client-18 lustre]# pwd
/usr/include/lustre
Ah, great! Good riddance.
The manual should have a note to users about lustre_idl.h's disappearance.
While doing the libcfs cleanup I discovered that while lustre_idl.h is not exported for user land in the rpms liblustreapi still depends on this header to build.
> While doing the libcfs cleanup I discovered that while lustre_idl.h is not exported for user land in the rpms liblustreapi still depends on this header to build.
That's intentional. lustre_idl.h will not be packaged. llapi is the public interface.
I was surprised that liblustreapi still required lustre_idl.h. As I cleanup the libcfs header dependencies I noticed that the lustre headers relating to the utils is just a mess. Many lustre kernel headers are being pulled into the utils library. There is no clear separation of the layers and headers are placed either in lustre/include or lustre/include/lustre for the utils. This will need to be cleaned up.
When I was looking through this before the developer workshop I also saw this issue with kernel headers included into userspace. However, most of those headers are not actually needed for liblustreapi to work. I should send you my WIP patch for cleaning up some of those dependencies. In some cases it means reimplementing llapi functions to use different APIs (e.g. getting the Lustre version), but that's OK and maybe even desirable. In other cases, we just need to disentangle the headers, and only include the needed headers into userspace, rather than generic ones like "libcfs.h".
That is excellent that some functions for liblustreapi has to be redone. We can move that code to liblustre_util.c and make it LGPL
Please do send you WIP patch. I will gladly move it forward. I myself started to unwind the libcfs.h mess for the user land tools and utilities.
With the work of LU-6401 I started to look closely at lustre_idl.h and I noticed its not really needed to user land except for wiretest and the wirechecker hack. It might be possible to make lustre_idl.h a kernel space only header.
What does IDL even stand for?
Since Lustre is based on an RCP interface, most likely it's a reference to Interface Definition Language, such as the kind used with rpcgen to use SUNRPC and similar RPC implementations.
Thanks, that makes sense. Since there is activity here, maybe someone can add that definition to the file in the next commit.
This ticket can be closed since we have test sanity 400b to make sure this doesn't happen again.