Details

    • Bug
    • Resolution: Fixed
    • Minor
    • Lustre 2.6.0
    • Lustre 2.6.0
    • 3
    • 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?

      Attachments

        Issue Links

          Activity

            [LU-5065] lfs should dogfood llapi
            jhammond John Hammond added a comment -

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

            jhammond John Hammond added a comment - Patch landed to master. lfs interactive mode stays for now.
            rread Robert Read added a comment -

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

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

            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.

            rhenwood Richard Henwood (Inactive) added a comment - 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.

            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.

            bogl Bob Glossman (Inactive) added a comment - 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.
            jhammond John Hammond added a comment - - edited

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

            jhammond John Hammond added a comment - - edited Please see http://review.whamcloud.com/10336 LU-5065 utils: uninclude lustre_idl.h from lfs.

            People

              jhammond John Hammond
              jhammond John Hammond
              Votes:
              1 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: