fix(ioctl): handle partial GetEntryInfoV2 response via GetEntryInfoResult field#317
Open
pravar3088 wants to merge 2 commits intomainfrom
Open
fix(ioctl): handle partial GetEntryInfoV2 response via GetEntryInfoResult field#317pravar3088 wants to merge 2 commits intomainfrom
pravar3088 wants to merge 2 commits intomainfrom
Conversation
The beegfs-core client module's GetEntryInfoV2 ioctl is updated to add a basicOnly field: when the inode is locked in the global lock store (e.g. chunk rebalancing in progress), the kernel now returns success with basicOnly=1 instead of EBUSY. Only the basic entry info fields (owner, entry/parent IDs, type, feature flags) are populated in that case; stripe pattern, PathInfo, RST, and session fields are left unset. This commit adds BasicOnly to the ioctl arg struct to match the updated kernel ABI, and return early with OpsErr_INODELOCKED in GetEntryInfoResponse.Result so callers can distinguish a partial result from a complete one.
Contributor
Author
|
I did some basic tests with fix from PR#4630 and changes seem to work fine. So I triggered a rebalance operation for a 10GB file: I put a watch on After rebalance was finished - I can again see fields like stripePattern, chunkSize etc. |
pravar3088
commented
Apr 17, 2026
| // above are valid, but stripe pattern, PathInfo, RST, and session fields are not populated. | ||
| // Return OpsErr_INODELOCKED in the response so callers can distinguish a partial result. | ||
| if arg.BasicOnly != 0 { | ||
| return entryInfo, msg.GetEntryInfoResponse{Result: beegfs.OpsErr_INODELOCKED}, nil |
Contributor
Author
There was a problem hiding this comment.
Not entirely sure about the design here. Currently, when arg.BasicOnly != 0, we return OpsErr_INODELOCKED in GetEntryInfoResponse.Result to signal a partial result — but this is technically an assumption: we know the ioctl returned partial data, not what the metadata server actually returned.
Options I could think of:
- Keep OpsErr_INODELOCKED — semantically correct today since basicOnly=1 is only set for INODELOCKED in the kernel, but it couples the Go layer to a kernel implementation detail
- Use a general/neutral error code — avoids the assumption, but I'm not sure what the right value would be here
- Just leave Result unset — callers can't explicitly distinguish partial data from a full result with no targets, but arguably the zero/default values themselves (e.g. no stripe targets, zero chunk size) already hint that full data wasn't fetched
Member
There was a problem hiding this comment.
What if we replace the BasicOnly boolean flag with a GetEntryInfoResult field that can be used to communicate the exact OpsErr back to the caller?
Then in the client code instead of having to check specific errors, we can always just report back any non-success rpcRes from FhgfsOpsRemoting_GetEntryInfo and let the caller decide what to do.
Contributor
Author
iamjoemccormick
added a commit
that referenced
this pull request
Apr 17, 2026
The new errors surfaced when using the `GetEntryInfoV2` ioctl and subsequent changes in #317 prompted the realization that `beegfs entry info` only worked before because `newEntry()` was never checking if `entryInfo msg.GetEntryInfoResponse` actually contained valid data (based on `entryInfo.Result`). At best this returned misleading output in CTL, at worst there are other places that make assumptions based on the validity of that data. Before when GetEntryInfoResponse failed (e.g., OpsErr_INODELOCKED during chunk rebalancing), fields derived from that response previously defaulted to Go zero values, causing consumers like 'beegfs entry info' to silently display misleading data (e.g., Chunksize: 0, Reading: 0, Writing: 0). This change extracts Pattern, Remote, NumSessionsRead, NumSessionsWrite, and FileState into a new EntryDetails struct. Entry.Details is a pointer that is nil when EntryInfoPopulated != SUCCESS, so: - Display functions gate all response-derived output behind a single nil check, showing (unavailable) instead of zero values. - Backend consumers (set, migrate, create, RST) get explicit nil guards that surface errors instead of silently operating on empty data. - Future fields added to EntryDetails are automatically handled by the display layer without requiring frontend changes. Also introduces ErrEntryDetailsUnavailable in the migrate/chooser path, replacing the previous behavior where a locked inode silently produced zero-value fields that triggered ErrEntryHasNoTargets. Assisted-by: Claude:claude-opus-4-6
9 tasks
Mirrors the core-side rename from basicOnly to getEntryInfoResult. The field now carries the exact FhgfsOpsErr code from the metadata RPC rather than a hardcoded boolean, so the partial result reason is propagated to callers without assumption.
pravar3088
force-pushed
the
fix/getentryinfo-v2-basic-only-go
branch
from
929f8df to
12f7689
Compare
pravar3088
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do / why do we need it?
Required for all PRs.
The beegfs-core client module's GetEntryInfoV2 ioctl exposes a
getEntryInfoResultfield inBeegfsIoctl_GetEntryInfoV2_Argthat carries the exactFhgfsOpsErrcode returned by the metadata RPC. When the RPC returns a non-success result, the kernel returns success from the ioctl with the error code in getEntryInfoResult; only the basic entry info fields are valid in that case.This PR adds the corresponding
GetEntryInfoResult uint8field to the Go ioctl arg struct and passes its value directly asbeegfs.OpsErr(arg.GetEntryInfoResult)in GetEntryInfoResponse.Result, letting callers distinguish a partial result from a complete one without any hardcoded error assumptions.Related Issue(s)
Required when applicable.
Where should the reviewer(s) start reviewing this?
Only required for larger PRs when this may not be immediately obvious.
Are there any specific topics we should discuss before merging?
Not required.
What are the next steps after this PR?
Not required.
Checklist before merging:
Required for all PRs.
When creating a PR these are items to keep in mind that cannot be checked by GitHub actions:
For more details refer to the Go coding standards and the pull request process.