[Lustre-devel] lustre-devel packaging
Bruce Korb
We at Xyratex are needing to develop some auxiliary utilities that
need internal data structures from lustre. To accomplish this, our
sources have been reaching into the lustre build tree for the
necessary headers. Not being a particular fan of that approach,
I developed a patch for lustre that will create another RPM:
a lustre-devel package.
It builds the packages and drops the headers into a tree that looks like
this:
usr/include/lustre
usr/include/lustre/darwin
usr/include/lustre/linux
usr/include/lustre/lnet
usr/include/lustre/lnet/darwin
usr/include/lustre/lnet/linux
usr/include/lustre/lnet/winnt
usr/include/lustre/libcfs
usr/include/lustre/libcfs/darwin
usr/include/lustre/libcfs/linux
usr/include/lustre/libcfs/winnt
usr/include/lustre/libcfs/util
usr/include/lustre/libcfs/posix
usr/include/linux
usr/include/libcfs/posix
one file is dropped into include/libcfs/posix and one file into
include/linux.
I'd have preferred not doing that, but the old code already did.
186 headers get installed.
The other files installed as part of lustre-devel are object
archives and library interface man pages:
usr/lib64/libcfsutil.a
usr/lib64/liblustreapi.a
usr/lib64/liblustre.a
usr/lib64/libptlctl.a
usr/lib64/libiam.a
usr/share/lustre/mpich-1.2.6-lustre.patch
usr/share/man/man3/llapi_quotactl.3.gz
usr/share/man/man3/llapi_file_open.3.gz
usr/share/man/man3/llapi_file_create.3.gz
usr/share/man/man3/llapi_file_get_stripe.3.gz
The rest of the lustre installation is left with the main package.
Below is my git-commit message. Under separate cover, I'll post the
git patch under the subject "[PATCH] lustre-devel packaging"
* autoMakefile.am
Constrain lines to 80 columns (readability)
Per automake maintainers, configurables should be set to a
make file macro before use. Enables "make" time replacement.
* build/autoMakefile.am.toplevel
Constrain lines to 80 columns (readability)
* libcfs/include/Makefile.am
Put every header under libcfs/include into nobase_pkginclude_HEADERS.
* lnet/autoconf/lustre-lnet.m4
The body of an m4 (autoconf) macro is indented (readability).
"LNET_MAX_PAYLOAD" is now a configured value
lnet/include/lnet/types.h is now a configured file
Alphabetize and indent the configured files
* lnet/include/Makefile.am
Collect the neaders into nobase_pkginclude_HEADERS and
nodist_nobase_pkginclude_HEADERS (just lnet/types.h)
* lnet/include/lnet/Makefile.am
lnet/include/lnet/types.h is now a configured file
* lnet/include/lnet/types.h
Renamed to:
* lnet/include/lnet/types.h.in
Add configured #define for LNET_MAX_PAYLOAD and adjust CPP testing.
* lustre.spec.in
354 columns in a single line is too much. Trim the egregious ones.
add a "%package devel" directive
In %install section, separate "devel" files from non-devel and list
them in lustre-devel.files and lustre.files.
* lustre/autoconf/lustre-core.m4
Add config.h #define for OBD_MAX_IOCTL_BUFFER and make it a
substitution value as well.
lustre/include/lustre/Makefile.am is removed. Do not configure.
* lustre/include/Makefile.am
The lustre/include/lustre/*.h headers get installed in pkginclude.
Drop the rest into nodist_pkginclude_HEADER (lustre_ver.h) or
nobase_pkginclude_HEADER (everything else).
* lustre/include/linux/Makefile.am
The EXTRA_DIST value is obsolete.
* lustre/include/linux/lustre_acl.h
Fix spelling of "Should not include directly".
* lustre/include/lustre/Makefile.am
Obsolete
* lustre/include/lustre_sec.h
Prefer "__u32" to "uint32_t". Eliminates need for inttypes.h.
* lustre/include/lustre_ver.h.in
Add configured #define OBD_MAX_IOCTL_BUFFER
* lustre/utils/Makefile.am
Move obd.c and lustre_cfg.c sources to convenience library.
---
autoMakefile.am | 36 ++-
build/autoMakefile.am.toplevel | 17 +-
libcfs/include/Makefile.am | 8 +
lnet/autoconf/lustre-lnet.m4 | 102 ++++----
lnet/include/Makefile.am | 10 +
lnet/include/lnet/Makefile.am | 2 +-
lnet/include/lnet/types.h | 509
-------------------------------------
lnet/include/lnet/types.h.in | 505
++++++++++++++++++++++++++++++++++++
lustre.spec.in | 156 ++++++++----
lustre/autoconf/lustre-core.m4 | 13 +-
lustre/include/Makefile.am | 71 +++++-
lustre/include/linux/Makefile.am | 7 -
lustre/include/linux/lustre_acl.h | 2 +-
lustre/include/lustre/Makefile.am | 42 ---
lustre/include/lustre_sec.h | 4 +-
lustre/include/lustre_ver.h.in | 23 +-
lustre/utils/Makefile.am | 12 +-
17 files changed, 814 insertions(+), 705 deletions(-)
=== 500 insertions and 500 deletions from file move.
=== patch actually 300 inserts and 200 deletes.
_______________________________________________
Lustre-devel mailing list
Lustre...@lists.lustre.org
http://lists.lustre.org/mailman/listinfo/lustre-devel
Andreas Dilger
> We at Xyratex are needing to develop some auxiliary utilities that
> need internal data structures from lustre. To accomplish this, our
> sources have been reaching into the lustre build tree for the
> necessary headers.
That is messy, I agree.
> Not being a particular fan of that approach, I developed a patch for
> lustre that will create another RPM: a lustre-devel package.
I like the idea of having a separate lustre-devel package, since this
is fairly standard practise for other libraries and such.
My main question would be - do you _need_ to have access to all of the
headers that you included, or did you simply include all of the headers
because that was the easiest thing to do? Doing a simple check on the
current master tree, it appears you just copied all of the headers in
libcfs/include, lnet/include, and lustre/include (which total 186 files).
By just copying everything into "public" headers, it is introducing a
maintenance nightmare, because it is no longer clear which APIs, structs,
constants, ioctls, etc. are private to Lustre or specific tools, and
which ones might possibly be used by external tools and would cause those
tools to break if they were changed for some reason.
> The other files installed as part of lustre-devel are object
> archives and library interface man pages:
>
> usr/lib64/libcfsutil.a
> usr/lib64/liblustreapi.a
> usr/lib64/liblustre.a
> usr/lib64/libptlctl.a
> usr/lib64/libiam.a
Now that you post this list of libraries, it in fact appears that libiam
is not even used by userspace at all anymore. From simple checking of
lustre/utils/Makefile.am, it only shows mkfs_lustre.c as a potential user,
and that doesn't seem to use it at all. The creation of the object index
"oi.16", which is the only IAM-format index in the filesystem, is done in
the kernel since before Lustre 2.0 was released.
So, rather than simply copying everything that is available, it would be
much better to have a discussion about what APIs you are using (or which
ones you wish would be available), and then implement llapi_* wrappers
for those interfaces. This would at least give some level of abstraction
from the low-level interfaces, and makes it much clearer which interfaces
are actually in use by tools that are not part of the Lustre package,
(and should try to remain relatively stable), and which interfaces, ioctls,
structures, etc. are for internal use only (or sometimes not in use at all).
> usr/share/lustre/mpich-1.2.6-lustre.patch
I think a significantly improved version this patch was already included
in the MPICH upstream release, and this one is obsolete.
> usr/share/man/man3/llapi_quotactl.3.gz
> usr/share/man/man3/llapi_file_open.3.gz
> usr/share/man/man3/llapi_file_create.3.gz
> usr/share/man/man3/llapi_file_get_stripe.3.gz
>
> Below is my git-commit message. Under separate cover, I'll post the
> git patch under the subject "[PATCH] lustre-devel packaging"
Looking at the descriptions, the patches look quite reasonable. However,
you need to submit patches to Gerrit in order to get them inspected,
tested, and landed. Please see:
http://wiki.whamcloud.com/display/PUB/Submitting+Changes
and in particular:
http://wiki.whamcloud.com/display/PUB/Using+Gerrit
There are a number of people at Xyratex already following this process
that you can likely ask for guidance.
Cheers, Andreas
--
Andreas Dilger
Principal Engineer
Whamcloud, Inc.
Bruce Korb
On 11/15/11 1:47 AM, Andreas Dilger wrote:
>On 2011-11-14, at 7:11 AM, Bruce Korb wrote:
>My main question would be - do you _need_ to have access to all of the
>headers that you included, or did you simply include all of the headers
>because that was the easiest thing to do? Doing a simple check on the
>current master tree, it appears you just copied all of the headers in
>libcfs/include, lnet/include, and lustre/include (which total 186 files).
Lets assume I did a minimal approach and only included the necessary
headers. Then, someone oblivious to which headers get exported and
which do not, then added a new dependency into the headers. Everything
builds and checks out so it looks right and you ship the new version.
Except it isn't because of the new inadvertent dependency. Oops.
The correct and proper solution is for each component of lustre to
explicitly copy interface headers into an include staging area
with everything under that getting installed. I'm not going there.
That is a future exercise. Probably ought to be done, though.
OK, *surely* ought to be done. :)
>By just copying everything into "public" headers, it is introducing a
>maintenance nightmare, because it is no longer clear which APIs, structs,
>constants, ioctls, etc. are private to Lustre or specific tools, and
>which ones might possibly be used by external tools and would cause those
>tools to break if they were changed for some reason.
Were the private vs. public headers separated in some way, then
the unwanted exports could be removed. At the moment, I think
the assumption has to be that clients of the lustre-devel package
would have to be friendly clients. Very friendly. Right now, we
have actual clients grubbing around all over the lustre source
tree in a very unfriendly way.
>> The other files installed as part of lustre-devel areŠ..
>So, rather than simply copying everything that is available, it would be
>much better to have a discussion about what APIs you are using (or which
>ones you wish would be available), and then implement llapi_* wrappers
Ultimately, that is completely correct. For now, I'm mostly interested in
getting headers and libraries installed in a way where I'm not dependent
upon the build tree layout. It is already understood that if lustre
changes
internals these utilities have to adapt. Adapting semi-private interfaces
to a coherent framework would be such a change. This change simply
isolates
our auxiliary utilities from changes in build layout. That's step 1.
I (we at Xyratex) would be completely okay with a usage caveat that the
interfaces exposed are subject to change while the process of working out
exported vs. completely private interfaces goes on.
>>usr/share/lustre/mpich-1.2.6-lustre.patch
>
>I think a significantly improved version this patch was already included
>in the MPICH upstream release, and this one is obsolete.
The .spec file needs to adapt to whatever gets staged into BUILDROOT.
Some of that may be hard coded and need changing, but the .spec file
ought to be as scripted as possible, minimizing the need for any changes
when the lustre subcomponents change the set of files they install.
>>
>> Below is my git-commit message. Under separate cover, I'll post the
>> git patch under the subject "[PATCH] lustre-devel packaging"
>
>Looking at the descriptions, the patches look quite reasonable. However,
>you need to submit patches to Gerrit in order to get them inspected,
>tested, and landed.
I was actually starting with email before going to a formal review request.
You have already seen the review request for changes I consider less
controversial (see "P.S." below), even if the changes were too unfocused
as a single patch.
>Cheers, Andreas
Thank you!! Regards, Bruce
P.S. the other issue (LU-483) got combined because of procedural issues.
Sorry about that. I will do as you ask within a few days and break it
up into several independent patches, but still under LU-483, yes?
Nathan Rutman
On Nov 15, 2011, at 7:22 AM, "Bruce Korb" <bruce...@xyratex.com> wrote:
> Hi Adreas,
>
> On 11/15/11 1:47 AM, Andreas Dilger wrote:
>
>> On 2011-11-14, at 7:11 AM, Bruce Korb wrote:
>> My main question would be - do you _need_ to have access to all of the
>> headers that you included, or did you simply include all of the headers
>> because that was the easiest thing to do? Doing a simple check on the
>> current master tree, it appears you just copied all of the headers in
>> libcfs/include, lnet/include, and lustre/include (which total 186 files).
>
>
> Were the private vs. public headers separated in some way, then
> the unwanted exports could be removed. At the moment, I think
> the assumption has to be that clients of the lustre-devel package
> would have to be friendly clients. Very friendly. Right now, we
> have actual clients grubbing around all over the lustre source
> tree in a very unfriendly way.
Actually I thought we were careful to only expose llapi_* functions. So what's doing the grubbing?
Bruce Korb
I did do a little more due diligence:
On 11/15/11 7:22 AM, Bruce Korb wrote:
>>My main question would be - do you _need_ to have access to all of the
>>headers that you included, or did you simply include all of the headers
>>because that was the easiest thing to do? Doing a simple check on the
>>current master tree, it appears you just copied all of the headers in
>>libcfs/include, lnet/include, and lustre/include (which total 186 files).
>
>Lets assume I did a minimal approach and only included the necessary
libcfs/libcfsutil.h
lnet/lnetctl.h
lustre/liblustreapi.h
lustre/lustre_idl.h
test.h
utils/obdctl.h
These are the headers directly #include-d by our "utility"
that live in the lustre tree. They likely pull in several more.
As I said elsewhere somewhere, unless the libcfs, lnet, lustre and utils
components of lustre know that these are exported via an unambiguous
mechanism, installing only the minimal subset will be prone to problems.
A better way is to have them stage the headers and run a validation that
they are all both self-sufficient and idempotent, meaning that any
requisite headers are also staged and they are all guarded with duplicate
inclusion guards. This can be done with a trivial script.
I can make this part of the "lustre-devel package" project. I was trying
to limit scope by installing everything in sight. At least for now.
Andreas Dilger
>
> I did do a little more due diligence:
>
> On 11/15/11 7:22 AM, Bruce Korb wrote:
>>> My main question would be - do you _need_ to have access to all of the
>>> headers that you included, or did you simply include all of the headers
>>> because that was the easiest thing to do? Doing a simple check on the
>>> current master tree, it appears you just copied all of the headers in
>>> libcfs/include, lnet/include, and lustre/include (which total 186 files).
>>
>> Lets assume I did a minimal approach and only included the necessary
>
> libcfs/libcfsutil.h
> lnet/lnetctl.h
> lustre/liblustreapi.h
> lustre/lustre_idl.h
> test.h
> utils/obdctl.h
>
> These are the headers directly #include-d by our "utility"
Including liblustrapi.h is expected, since this is the entry point for the Lustre wrappers, and this header is already installed.
I did some work several months ago to make lustre_idl.h usable from userspace for lfsck, which works OK except for the use of __u32/__u64 and friends, which needs the "types.h" header to define.
I haven't looked at the other headers (just about to get on a plane) but I think they might be acceptable for low-level Lustre specific applications. That said, the only previous user of these headers is probably lctl, and I'm hesitant to expose them as an "API".
As Nathan wrote, we've tried in the past to do the right thing and create llapi wrappers for code that needs to poke into the Lustre kernel interfaces. This allows things like e.g. fixing the ioctl numbers, changing the data structures used with the kernel, etc. without having to modify the applications that are using these wrappers.
I'm the first one to admit that the llapi_* wrappers do not address a large number of use cases, but that also won't get better if nobody works to improve them.
I guess a reasonable question at this point is what specifically you are trying to access? Is it (essentially) trying to link directly into lctl, lfs, etc? Access to wire protocol structures (that lustre_idl.h should handle), or something else entirely?
Alexey Lyashkov
On Nov 17, 2011, at 05:35, Andreas Dilger wrote:
> On 2011-11-16, at 14:22, Bruce Korb <bruce...@xyratex.com> wrote:
>>
>> I did do a little more due diligence:
>>
>> On 11/15/11 7:22 AM, Bruce Korb wrote:
>>>> My main question would be - do you _need_ to have access to all of the
>>>> headers that you included, or did you simply include all of the headers
>>>> because that was the easiest thing to do? Doing a simple check on the
>>>> current master tree, it appears you just copied all of the headers in
>>>> libcfs/include, lnet/include, and lustre/include (which total 186 files).
>>>
>>> Lets assume I did a minimal approach and only included the necessary
>>
>> libcfs/libcfsutil.h
>> lnet/lnetctl.h
>> lustre/liblustreapi.h
>> lustre/lustre_idl.h
>> test.h
>> utils/obdctl.h
>>
>> These are the headers directly #include-d by our "utility"
>
> Including liblustrapi.h is expected, since this is the entry point for the Lustre wrappers, and this header is already installed.
>
> I did some work several months ago to make lustre_idl.h usable from userspace for lfsck, which works OK except for the use of __u32/__u64 and friends, which needs the "types.h" header to define.
that should be done via libcfs.h i think ?