[LU-4997] lustre_idl.h compilation regression from LU-2743

• Status: Closed
• Project: Lustre
• Affects Version/s: Lustre 2.5.1
• Fix Version/s: Lustre 2.6.0
• Type: Bug
• Priority: Critical
• Reporter: Christopher Morrone
• Assignee: Li Wei (Inactive)
• Resolution: Fixed
• Severity: 3

Description

LU-2743 introduced a regression into lustre_idl.h. One reason that it no longer compiles in installed user space because of this addition:

#include <lustre/lustre_errno.h>

But lustre_errno. is not packaged.

There are two questions that need answering:

• Should lustre_idl.h really include lustre_errno.h?
• Should lustre_errno.h really be a new interface exposed to user space?

If the answers to both are "yes", we can consider packaging lustre_errno.h.

Of course, we also need to decide if we will continue packaging luste_idl.h (see parent issue LU-1606). If we stop packaging it, then this ticket is moot.

Issue Links

• is blocking: LU-5011 lustre_idl.h again does not compile i...

Comments

[John Hammond]

Please see http://review.whamcloud.com/10209.

[Peter Jones]

Assigning to Li Wei as the original author of this code but perhaps this can be closed as a duplicate of the ticket John H is working

[Li Wei (Inactive)]

My thoughts were (and still are):

• The contents of lustre_errno.h are part of the wire protocol. If there were not so many error codes, I would have defined them directly in lustre_idl.h.
• The contents of lustre_errno.h should not be needed by user space code, as the client kernel drivers should have translated the wire error codes back to host error codes.
• The rule seems to be that only lustre_user.h should be available to user space code.

[Li Wei (Inactive)]

By "user space code", I don't intend to include any user space implementations of Lustre client, of course.

[John Hammond]

> The rule seems to be that only lustre_user.h should be available to user space code.

That rule has not been followed.

Nothing in lustre_idl.h uses any definitions from lustre_errno.h. What's the matter with "include waht you use." Why should lustre_idl.h include something it doesn't need?

[Li Wei (Inactive)]

As to the rule, are current violations too hard to fix?

Yes, I agree in principle a header should not include another one that it doesn't depend on. However, the case here is a bit different: The error definitions should be defined in lustre_idl.h if there were not so many of them. Also, they are actually conceptually used by pb_status and hpk_errval in lustre_idl.h.

[Andreas Dilger]

Li Wei,
in a related note, are the errno values that Lustre is using on the network checked in wirecheck.c?

[Li Wei (Inactive)]

Andreas, no, I did not add such checks. Could you explain a bit about your thought behind the question?

[John Hammond]

lustre_idl.h is no longer packaged for use outside of the Lustre tree.

[Li Wei (Inactive)]

That's cool! Thanks, John.