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
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.
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
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)
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.
And if I want to use any other parser on the face of the planet, what then?
Exactly, which means it assumes that the fidstr was formatted using DFID.
llapi_fid2path uses the other macros for the scanning form, SFID/RFID, not DFID/PFID. 
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.
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.
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?