-
Notifications
You must be signed in to change notification settings - Fork 39
Provide idiomatic abstraction for vGPU type information #68
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
8179da6
to
b1fc692
Compare
Ping. Is this of any interest for this library? |
@christoph-heiss Thanks for the ping. A bit of background is that we (IOP Systems) became the primary maintainer of this repo earlier this year, after the original owner took a job that made it tricky for them to contribute to OSS. This PR was submitted before we came on board, and fell through the crack a bit during this transition. We will take a look and hopefully get it merged soon. |
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've got two nits here but otherwise this looks good to me. I'm still getting up to speed on the codebase here so I'm going to get @brayniac to give a second opinion here.
b1fc692
to
added4f
Compare
Thanks for the review! I've fixed both nits. |
added4f
to
bc9edae
Compare
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
bc9edae
to
f27405c
Compare
Pretty straight forward overall. Introduces a new
VgpuType
struct, which wraps over anvmlVgpuTypeId_t
and a proper idiomatic way to access these informations.I'v also added a
Nvml::lib()
method, for accessing the underlying (raw) library from the abstraction. This would be useful for e.g. calling functions which are not yet abstracted innvml-wrapper
, without having to completely usenvml-wrapper-sys
and NIH, basically. Hopefully that is acceptable.