-
Notifications
You must be signed in to change notification settings - Fork 319
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Extract FileMode from host path if possible #960
base: main
Are you sure you want to change the base?
Conversation
dn, err := devices.DeviceFromPath(path, "rwm") | ||
if err != nil { | ||
// return nil, fmt.Errorf("failed to get device information for %q: %w", path, err) | ||
return nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To what will nil
translate to in the spec? => 0 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nil
would be the current behaviour. In the case of runc
this is filled in by the runtime and read from the host. In the case of kata, this would be 0 as you've observed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The interface is the CDI spec, so how does runc know this value is nil? It is not encoded in the CDI spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, maybe runc
is not the correct reference here. For regular devices, a higher-level runtime such as containerd also extracts the FileMode (see https://github.com/containerd/containerd/blob/9deb0b8f748ee038dfd48c51d7aee6b55e34e9c1/pkg/oci/utils_unix.go#L142). This is not done for CDI devices since the code from https://github.com/cncf-tags/container-device-interface/blob/012c8be38d9444f08b7e6eeedbcddd733adf8632/pkg/cdi/container-edits_unix.go#L60 is used for this instead.
This means that for OCI devices created from a CDI spec, this field is nil
if unspecified, whereas for "regular" devices this field is populated from the device node on the host.
@klihub do you know why we elected to not add information other than the Major
and Minor
numbers to the devices in CDI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@elezar Do you mean what is the reason for not filling in any other missing information wrt. the CDI Spec DeviceNode
than type
, major
, and minor
? TBH, I don't recall any specifics, but I suspect it's only because that's the minimum working implementation.
Signed-off-by: Evan Lezar <[email protected]>
eb70273
to
4dae5ae
Compare
This change extracts the filemode from the host device nodes for CDI device specs. This allows the spec to be used in cases where a low-level runtime such as
runc
does not query the existing device node in the same way.