[LU-5000] lustre_idl.h compilation regression from ORNL-10

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.4.0, Lustre 2.4.1, Lustre 2.5.0, Lustre 2.4.2, Lustre 2.5.1, Lustre 2.4.3
• Type: Bug
• Priority: Minor
• Reporter: Christopher Morrone
• Assignee: John Hammond
• Resolution: Duplicate
• Severity: 3

Description

Commit d190c63f, "ORNL-10: Basic IR implementation" broke the compilation of lustre_idl.h in user space. lustre_idl.h is packaged for installation in user space. We cannot rely on lustre's internals to be available when lustre_idl.h is compiled.

In particular, the bug is here:

struct mgs_nidtbl_entry {
        __u64           mne_version;    /* table version of this entry */
        __u32           mne_instance;   /* target instance # */
        __u32           mne_index;      /* target index */
        __u32           mne_length;     /* length of this entry - by bytes */
        __u8            mne_type;       /* target type LDD_F_SV_TYPE_OST/MDT */
        __u8            mne_nid_type;   /* type of nid(mbz). for ipv6. */
        __u8            mne_nid_size;   /* size of each NID, by bytes */
        __u8            mne_nid_count;  /* # of NIDs in buffer */
        union { 
                lnet_nid_t nids[0];     /* variable size buffer for NIDs. */
        } u;
};

lnet_nid_t is not defined in lustre_idl.h.

Issue Links

• is blocking: LU-5011 lustre_idl.h again does not compile i...
• is related to: LU-6401 Untangle lustre userland and kernel h...
• is related to: LU-6245 Untangle userland and kernel space su...

Comments

[Peter Jones]

Assigning to Jinshan as the original author of this work

[Andreas Dilger]

John, I thought I recall you submitting a patch to fix this issue? I can't recall the patch number, however.

[John Hammond]

I abandoned that patch since we no longer package lustre_idl.h for userspace. The header is still broken in the sense that it uses lnet_nid_t but does not directly include lnet/lnet.h or lnet/types.h. However it does include libcfs/libcfs.h which includes libcfs/libcfs_private.h which includes lnet/types.h (which itself includes libcfs/libcfs.h).

[Christopher Morrone]

To put it another way: this ticket needs to remain open to deal with the remaining technical debt. However, since lustre_idl.h is no longer packaged for user space, this ticket no longer necessarily need be fixed before the 2.6 release.

We might want to fix it before 2.6 because we are worried about having external data types polluting the wire protocol definitions. But I'll let you folks decide that.

[Andreas Dilger]

I think it makes sense to move the non "type" definitions out of <lnet/types.h> and into <lnet/lnet.h>, and possibly consolidate this with <lnet/lib-types.h> to have a single header that contiains the wire protocol structures and definitions, and this can be included by <lustre/lustre_idl.h> without dragging in a bunch of unrelated stuff.

Since we are not exporting lustre_idl.h to userspace anymore, this kind of code reorg should be deferred to 2.7.

[James A Simmons]

With patch http://review.whamcloud.com/#/c/13792 all the wire protocol for lnet has been moved to lnet/types.h which is used by both user land and kernel space. Both types.h and lib-dlc.h are automatically included in lnet.h which is the master user land header to use. Since lustre_idl.h uses lnet/types.h by default it will be automatically fixed by this patch.

[James A Simmons]

Patch 13792 has landed so the lnet headers are now sorted so you have kernel only headers and user land only headers. So lustre_idl.h will not pull in a bunch of kernel cruft anymore from LNet. This ticket can be closed. Please note lustre_idl.h still pulls in kernel cruft from internal lustre kernel headers. That will be addressed under LU-6401

[James A Simmons]

Peter this ticket can be closed.

[Peter Jones]

ok - thanks James

[Peter Jones]

Oh! I assume that the original reporter (Chris) will speak up if he has a different viewpoint and we can reopen.