[LU-5065] lfs should dogfood llapi

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.6.0
• Affects Version/s: Lustre 2.6.0
• Labels: lfs llapi
• Severity: 3
• Assignee: John Hammond
• Reporter: John Hammond

Description

As a means to improving/maintaining the usability of llapi (the library defined by lustre/include/lustre/lustreapi.h) I propose that lustre/utils/lfs.c be allowed only to depend on that header and other non-Lustre, non-libcfs, non-lnet headers. This will help us to ensure that the definitions needed to properly use the functions/functionality exported in by llapi are also present. This will require us moving some definitions from lustre_idl.h to lustre_user.h. (I would prefer that llapi properly encapsulate the lustre ioctls and that the contents of lustre_user.h not be exposed via lustreapi.h but proposal to move us in this direction met with disapproval, see the discussion on LU-5011.)

Currently lfs depends on the following headers:

/* For dirname() */
#include <libgen.h>

#include <lnet/lnetctl.h>

#include <libcfs/libcfs.h>
#include <lustre/lustre_idl.h>
#include <lustre/lustreapi.h>

#include <libcfs/libcfsutil.h>
#include "obdctl.h"

Here are some goals for people to complain about:

1. Move everything in lustre_idl.h that is needed to build lfs to lustre_user.h.
2. Add the following gem to the end of lfs.c:

   #ifdef _LUSTRE_IDL_H_
   # error "lfs should not include lustre_idl.h"
   #endif
   

3. Add similar preprocessor trickery to ensure that we're not depending on config.h.
4. Remove libcfsutil.h. Does anyone actually use the interactive mode of lfs? I've only done so by accident.

lfs also does some mystery cruft with ptl_initialize(), obd_initialize(), obd_finalize() which I believe is only to support llapi_ping_target(). Perhaps this could be moved to lctl, or into llapi, or rewritten to use /proc (or /sys), or deleted entirely. Can anyone offer a convincing explanation as to why users should (be allowed to) ping targets in the first place?

Issue Links

• is related to: LU-5011 lustre_idl.h again does not compile in user space

Comments

[John Hammond]

Patch landed to master. lfs interactive mode stays for now.

[Robert Read]

+1 for deleting interactive mode or at least not being the default option (which could be move to a different ticket).

[Richard Henwood (Inactive)]

I am in agreement with Bob on the point that we can live without it.

My experience is that interactive mode is an irritation that I exit from immediately. I am in favour of it's removal in pursuit of your 'dog fooding llapi' plan.

[Bob Glossman (Inactive)]

I object to item 4. in your goals list. I do use the interactive mode, at least occasionally. I'm not saying it's essential and we can't live without it, but I do say it's useful & I do use it. Even for non-interactive use it's sometimes more convenient to create a short multiline script and feed it to the tool with input redirection instead of doing many separate invocations of lfs. The same is true for lctl, which also has an interactive mode.

[John Hammond]

Please see http://review.whamcloud.com/10336 LU-5065 utils: uninclude lustre_idl.h from lfs.