-
Notifications
You must be signed in to change notification settings - Fork 138
Introduce krun_has_feature() API #525
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
base: main
Are you sure you want to change the base?
Conversation
|
Is this even a good idea? I mean an alternative could be checking if the related symbol names exists with dlysm, but we are not always constituent on whether the function exists or just returns ENOTSUP or doesn't even return any error at all e.g. krun_set_gpu_options |
f469a26 to
072693c
Compare
src/libkrun/src/lib.rs
Outdated
| "TEE" => cfg!(feature = "tee"), | ||
| "SEV" => cfg!(feature = "amd-sev"), | ||
| "TDX" => cfg!(feature = "tdx"), | ||
| "NITRO" => cfg!(feature = "nitro"), |
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.
#521 changes NITRO to AWS_NITRO, perhaps that could be modified here?
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.
Yeah, that is nicer.
Hmm, perhaps we should we also change these for consistency?
TDX -> INTEL_TDX
SEV -> AMD_SEV
I mean we should probably change the name in the Makefile too, but this seems like a more user friendly naming convention.
jakecorrenti
left a comment
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.
I like this idea and it reminds me of KVM's feature/extension API.
Rather than using strings, I wonder if we should do something similar to KVM where we have something along these lines:
// features are integers exposed by a public constant
#define KRUN_FEATURE_NET 0
#define KRUN_FEATURE_BLK 1
...
#define KRUN_FEATURE_VIRGL_RESOURCE_MAP2 10
// user calls API to check feature using the constant instead of hard-coded string
int ret = krun_has_feature(KRUN_FEATURE_NET);072693c to
b8a6dc7
Compare
Yeah, I've changed it to use integers. |
Introduce a function to query at runtime whether a specific feature was enabled at build time. This allows applications to check for optional capabilities before attempting to use them. This is especially useful for our tests and examples. Signed-off-by: Matej Hrica <mhrica@redhat.com>
b8a6dc7 to
b8c6efc
Compare
Introduce a function to query at runtime whether a specific feature was enabled at build time. This allows applications to check for optional capabilities before attempting to use them. This is especially useful for our tests and examples.