[LU-6401] Untangle lustre userland and kernel headers

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.8.0
• Fix Version/s: Lustre 2.11.0
• Type: Improvement
• Priority: Minor
• Reporter: James A Simmons
• Assignee: Ben Evans (Inactive)
• Resolution: Fixed
• Labels: patch, upstream
• Epic/Theme: upstream

Description

Much like the current situation with libcfs the headers for lustre being used internally for the kernel and for userland utilites is heavly intertwined. This work will look to unravel this mess and create a clear separation between lustre kernel space and lustre user land.

Issue Links

• is blocked by: LU-6245 Untangle userland and kernel space su...
• is related to: LU-9767 FS name checking got removed
• is related to: LU-9679 Prepare lustre for adoption into the ...
• is related to: LU-6202 clean up ioctl handling
• is related to: LU-6746 move IT_* definitions to lustre_idl.h
• is related to: LU-5000 lustre_idl.h compilation regression f...
• is related to: LU-6450 most MAY_* constants defined in lustr...
• is related to: LU-9128 remove OBD_OCD_VERSION-ed out code
• is related to: LU-9301 Ubuntu1404 client sanity test_400a/b ...
• is related to: LU-5011 lustre_idl.h again does not compile i...
• is related to: LU-8945 Stop building ptlrpc/layout.c in user...

Comments

[Jian Yu]

Hi James,
Are you going to submit patches for this ticket? If yes, I'll assign this ticket to me to take care of the patches.

[James A Simmons]

Yes soon but they are not ready. Wanted this ticket has a place holder until then.

[Ben Evans (Inactive)]

I'm working on a bunch of changes for this. What would be the best way of submitting them, the deltas here are pretty large (mostly cutting+pasting).

[Jian Yu]

Hi Ben,
FYI, Andreas had a suggestion in https://jira.hpdd.intel.com/browse/LU-6142?focusedCommentId=104684&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-104684

[James A Simmons]

Thank you for doing this work. Please break up the patches as small as possible where each patch does one specific change. The reason for this is these patches will be going upstream and very large patches that doing many things will be rejected since it is hard to inspect such a patch. Also any tabs or style changes should be patches in themselves.

Don't be afraid to push what you have now. I will gladly review your patches.

[Gerrit Updater]

Ben Evans ([e-mail]) uploaded a new patch: http://review.whamcloud.com/15952
Subject: LU-6401 headers: Create single .h for lu_seq_range
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 8f8350e1f849e271b09b1272f20a33a5f4f60c3d

[James A Simmons]

Ben looking at the patch could you place that new header in lustre/include/lustre/* since it used by wiretest which is a user land app.

[Ben Evans (Inactive)]

No problem, reworking the patch because it didn't get submitted correctly. Moving it to lustre/include/lustre is no problem.

[Ben Evans (Inactive)]

Given the comments in the patch I created from John Hammond, how should I proceed here? Giant include files like lustre_idl.h and lustre_user.h are pretty clunky, and really don't keep things organized in any easily maintainable and understandable way, and they tend to be the "easy out" for development (just include the one giant thing and be done with it).

My idea for separating things into files by function, and including all the "on the wire" data structures #include'd there seems to me to be a good way of cleaning up the code, and making it more maintainable.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/15952/
Subject: LU-6401 headers: Create single .h for lu_seq_range
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 1ecf5bd5a3bece3d615d037a332792e360b49a09

[James A Simmons]

I wish this was fixed but a lot more work needs to be done. Could you reopen this ticket and remove the Fix Version field.

[Joseph Gmitter (Inactive)]

James,
My mistake - I thought the rest of the work was occurring in the related LU tickets. Will revert my changes.
Joe

[Gerrit Updater]

Ben Evans ([e-mail]) uploaded a new patch: http://review.whamcloud.com/16339
Subject: LU-6401 headers: move swab functions to new header files
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 6b9113fef73419a49841d2d3c096c36cf9a26c75

[Ben Evans (Inactive)]

Should I create separate LUs for each one of these patches I do? I expect there to be a long series of them.

[James A Simmons]

That would be a lot of patches. I think it is okay to keep them under this ticket.

[James A Simmons]

Building some of test apps I hit LU-5011 due to libcfs/types.h being in lustre_idl.h. Ben I would be very grateful if you reworked lustre_idl.h to remove libcfs/types.h. If you do please place the work under LU-5011.

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: http://review.whamcloud.com/16494
Subject: LU-6401 lustre: make lustre_user.h compile in user space
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 001375a9a215eeb5fd773851021720728e91c670

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/16494/
Subject: LU-6401 lustre: make lustre_user.h compile in user space
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 9f19e2f152e1012558f37f6eecd336d516ff812e

[Gerrit Updater]

Ben Evans ([e-mail]) uploaded a new patch: http://review.whamcloud.com/16917
Subject: LU-6401 headers: Create a header for obdo related functions
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: fe73484a122a4c7df6528c0f451dec2d8f74a3e5

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: http://review.whamcloud.com/17720
Subject: LU-6401 uapi: change uapi_kernelcomm.h to LGPLv2.1 license
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 603dc605d666a2ba577817798d1a785266a66fcf

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/17720/
Subject: LU-6401 uapi: change uapi_kernelcomm.h to LGPLv2.1 license
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: c683c00408bc56bb68ab44d8bd21eaceb75fe86a

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/16339/
Subject: LU-6401 headers: move swab functions to new header files
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: adfb405433aac3691bbd00ed548203c57ce61e3a

[Gerrit Updater]

Ben Evans ([e-mail]) uploaded a new patch: http://review.whamcloud.com/18863
Subject: LU-6401 headers: move seq_range.h to correct directory
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: fca545541496f8ebe38fb9956fb1e80c71a4b9fa

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/16917/
Subject: LU-6401 headers: Create a header for obdo related functions
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 713174908cb8e5e3ceadd3ca1cb42a88b200e576

[Gerrit Updater]

Ben Evans ([e-mail]) uploaded a new patch: http://review.whamcloud.com/19266
Subject: LU-6401 obdclass: make get/set_wire_obdo non-inline
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: b34ee42a6b08b34c361bd7ce0d4c5ea46db3e912

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/18863/
Subject: LU-6401 headers: move seq_range.h to correct directory
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 1230bc483cbd15280ef2abf235eed600d500b9da

[Gerrit Updater]

Ben Evans ([e-mail]) uploaded a new patch: http://review.whamcloud.com/20120
Subject: LU-6401 headers: Move object_update functions to new header
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: d47505fdb75369a07ea06c844d2fc23ed248294a

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/19266/
Subject: LU-6401 obdclass: make get/set_wire_obdo non-inline
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 219e0a6dd4306a7424dd33146963a0a830035128

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/20120/
Subject: LU-6401 headers: Move object_update functions to new header
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 86068ab4d84dd0915fc91dbf59ac924ced9373c3

[Jian Yu]

Hi Ben,
So far, all of the existing patches for this ticket have landed. Are there any more new patches?

[James A Simmons]

Its far from being done.

[Ben Evans (Inactive)]

There is a lot left to do. I'll indicate when this process is done.

[Jian Yu]

Thank you James and Ben.

[Gerrit Updater]

Ben Evans ([e-mail]) uploaded a new patch: http://review.whamcloud.com/21484
Subject: LU-6401 headers: Move functions out of lustre_idl.h
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 9b6f392899f6a72d68af9d845516025aab38ec2b

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/21484/
Subject: LU-6401 headers: Move functions out of lustre_idl.h
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 2714d02cfd570b9fadb5b66483160f1c56e3c5fe

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: http://review.whamcloud.com/22522
Subject: LU-6401 header: remove assert from interval_set()
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: ae0502739905ca6d5745d4c957fb336b6e4cc42b

[Gerrit Updater]

Ben Evans ([e-mail]) uploaded a new patch: http://review.whamcloud.com/22712
Subject: LU-6401 headers: move lu_fid, ost_id funcs out of lustre_idl
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 494b366ca0e95cb8f3fed4dd81a5f0afe803be7c

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/22522/
Subject: LU-6401 header: remove assert from interval_set()
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 5207c488239e30e9aee51531ec3a363c649211de

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/24323
Subject: LU-6401 header: remove libcfs.h in interval_tree.h
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 8e8109538c140a76a598b5e44c46f433ac904030

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/24325
Subject: LU-6401 header: turn lustre_param.h into a proper UAPI header
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 89c9fe889d6eb92e31034ffa9c93fb29b305ea8f

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/24374
Subject: LU-6401 header: don't make lustre_acl.h a UAPI header
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 0cdf6cd1630811b59233736ac2ddd8d487d01ac1

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/22712/
Subject: LU-6401 headers: move lu_fid, ost_id funcs out of lustre_idl.h
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 51828ff66904be2a56a8790c3c9ce562a7c8e2e6

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/24323/
Subject: LU-6401 header: remove libcfs.h in interval_tree.h
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 24bc033262d15ab565f1a424c8e1a709993814d1

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/24568
Subject: LU-6401 header: turn lustre_ioctl.h into a proper UAPI header
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: c5b4be52a74be44c1a32acfe3ebb388b00003278

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/24569
Subject: LU-6401 headers: fix up lustre_ostid.h
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: d156b6fdff64bd4531a29c494c0f85f9782bf1fb

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/25194
Subject: LU-6401 uapi: split lustre_disk.h into two headers
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 97913453dcd56d8f9c9489f3e352030f6b212c3f

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/25246
Subject: LU-6401 uapi: remove lustre_idl.h from most userspace code
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: b7338d35902eff236392f1499a8bbd8eb4099e8a

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/25194/
Subject: LU-6401 uapi: split lustre_disk.h into two headers
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 3344e648236778565a8838ca85c655446ee7028c

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/24568/
Subject: LU-6401 uapi: turn lustre_ioctl.h into a proper UAPI header
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: b8afd0dcd3834a88b0ceead18c71d37052a00a8d

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/24569/
Subject: LU-6401 uapi: fix up lustre_ostid.h and lustre_fid.h
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 34acfbc2bfe502d18c12ba35771bde7c4a0f7906

[Gerrit Updater]

James Simmons ([e-mail]) uploaded a new patch: https://review.whamcloud.com/26966
Subject: LU-6401 uapi: change lustre_cfg.h into a proper UAPI header
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 5c019c4dc0f8e7b281b9d6751af98158b57a50d3

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/24325/
Subject: LU-6401 uapi: turn lustre_param.h into a proper UAPI header
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 424e56abd685c5eabc6a276154dec1c031cf5044

[James A Simmons]

One outstanding patch and I have the final one in the works. Almost done but I'm down to one last issue. For lustre_ioctl.h which is an UAPI headers to have CONFIG_LUSTRE_OBD_MAX_IOCTL_BUFFER is a big no no. So I see two ways this can be handled. One is make OBD_MAX_IOCTL_BUFFER a constant or make this a module parameter. Which approach should be done?

[Andreas Dilger]

Can you explain what is wrong with that parameter? I honestly don't think we'll change it any time soon, so it could be moved to an internal header. That said, what is wrong with exposing this constant to userspace?

[John Hammond]

0 lustre/include/uapi/linux/lustre_ioctl.h <global>               65 #define OBD_MAX_IOCTL_BUFFER CONFIG_LUSTRE_OBD_MAX_IOCTL_BUFFER
1 lustre/obdclass/linux/linux-module.c     obd_ioctl_getdata     167 if (hdr.ioc_len > OBD_MAX_IOCTL_BUFFER) {
2 lustre/obdclass/linux/linux-module.c     obd_ioctl_getdata     169 hdr.ioc_len, OBD_MAX_IOCTL_BUFFER);
3 lustre/utils/liblustreapi.c              cb_migrate_mdt_init  3853 char raw[OBD_MAX_IOCTL_BUFFER] = {'\0'};
4 lustre/utils/liblustreapi.c              llapi_obd_fstatfs    4167 char raw[OBD_MAX_IOCTL_BUFFER] = {'\0'};
5 lustre/utils/obd.c                       jt_obd_lov_getconfig 2219 desc.ld_tgt_count =
                                                                     ((OBD_MAX_IOCTL_BUFFER-sizeof(data)-sizeof(desc)) /

In cb_migrate_mdt_init() and llapi_obd_fstatfs() use of OBD_MAX_IOCTL_BUFFER can easily be replaced by using dynamic allocation from obd_ioctl_pack().

lctl lov_getconfig should just be deleted. But if that's not politically feasible then all of the information it prints can be gotten from lov proc.

Then there are no more uses of OBD_MAX_IOCTL_BUFFER in userspace.

[James A Simmons]

Last two patches have been updated that will finish this ticket !!!!

[Jian Yu]

Thank you James!

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/26966/
Subject: LU-6401 uapi: change lustre_cfg.h into a proper UAPI header
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 4fedbc879571305a0d14a2caa989c8c306efc54c

[Chris Horn]

Was it intentional to remove the 8 character filesystem name limit in commit 424e56abd685c5eabc6a276154dec1c031cf5044 ?

 

And if so, is there an LUDOC for that change?

[Chris Horn]

If that change was intentional then there's a bug with mkfs.lustre rename option that restricts a rename to 8 characters.

[Cory Spitz]

8 vs 16 length issue reported at LU-9767.

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/25246/
Subject: LU-6401 uapi: migrate remaining uapi headers to uapi directory
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 6712478e79588e73e28c7ccac3afc7ac2368a4f3

[James A Simmons]

After 3 years of work this is complete.

[Colin Faber [X] (Inactive)]

And nearly 3 years later it's still not closed =)

[James A Simmons]

Oops. Forgot to close it out.