Details
-
Bug
-
Resolution: Fixed
-
Major
-
Lustre 2.4.0, Lustre 2.4.1, Lustre 2.5.0, Lustre 2.4.2, Lustre 2.5.1, Lustre 2.4.3
-
3
-
13871
Description
New ticket for the the recurring problem of lustre_idl.h not compiling in user-space. Conversation began in LU-1606, but we want to have a separate ticket for Lustre 2.6.0.
Attachments
Issue Links
- is blocked by
-
-
- Resolved
-
-
-
- Resolved
-
-
-
- Closed
-
-
-
- Closed
-
- is related to
-
-
- Resolved
-
-
-
- Closed
-
-
LU-6245 Untangle userland and kernel space support for libcfs
-
- Resolved
-
- is related to
-
-
- Resolved
-
-
-
- Closed
-
-
-
- Resolved
-
Activity
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
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 ~]#
> 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.
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.
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.
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.
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.
> 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'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.
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.
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?