[LU-5065] lfs should dogfood llapi Created: 15/May/14  Updated: 03/Jun/14  Resolved: 03/Jun/14

Status: Closed
Project: Lustre
Component/s: None
Affects Version/s: Lustre 2.6.0
Fix Version/s: Lustre 2.6.0

Type: Bug Priority: Minor
Reporter: John Hammond Assignee: John Hammond
Resolution: Fixed Votes: 1
Labels: lfs, llapi

Issue Links:
Related
is related to LU-5011 lustre_idl.h again does not compile i... Closed
Severity: 3
Rank (Obsolete): 13991

 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?



 Comments   
Comment by John Hammond [ 15/May/14 ]

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

Comment by Bob Glossman (Inactive) [ 15/May/14 ]

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.

Comment by Richard Henwood (Inactive) [ 15/May/14 ]

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.

Comment by Robert Read (Inactive) [ 15/May/14 ]

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

Comment by John Hammond [ 03/Jun/14 ]

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

Generated at Sat Feb 10 01:48:14 UTC 2024 using Jira 9.4.14#940014-sha1:734e6822bbf0d45eff9af51f82432957f73aa32c.