Conversation
FreeMasen
force-pushed
the
feat/some-vgpu-apis
branch
4 times, most recently
from
2b5367f to
f3f0229
Compare
FreeMasen
force-pushed
the
feat/some-vgpu-apis
branch
from
f3f0229 to
970027b
Compare
FreeMasen
left a comment
FreeMasen
left a comment
There was a problem hiding this comment.
I believe this is ready for an initial review to make sure I am not doing anything that goes against the goals of this project. I need to dig a little deeper into how testing works for this project to add additional test for these additions.
I am going to leave the Draft status until I can get through tests, any feedback would be welcome!
| pub hour: u8, | ||
| pub min: u8, | ||
| pub sec: u8, | ||
| pub status: u8, |
There was a problem hiding this comment.
I couldn't find any documentation about why are expected values here and left the type effectively unchanged. Let me know if there is something I missed and I can add another enum to capture the status.
| impl TryFrom<nvmlVgpuMetadata_t> for VgpuMetadata { | ||
| type Error = NvmlError; | ||
| fn try_from(value: nvmlVgpuMetadata_t) -> Result<Self, Self::Error> { | ||
| let convert_c_str = |c_str: &[c_char]| { |
There was a problem hiding this comment.
This one is a bit odd, I wanted to avoid using any unsafe and so this will manually copy the string byte by byte which may not be the most efficient. Let me know if you'd prefer a different method for this or even if lossy conversion is acceptable here
|
I'm happy to review this once it is ready. Once you're happy with it lmk. |
FreeMasen
force-pushed
the
feat/some-vgpu-apis
branch
from
afce533 to
c6b1acb
Compare
| let mut count = 0; | ||
| unsafe { | ||
| nvml_try_count(sym(self.instance, std::ptr::null_mut(), &mut count))?; | ||
| metadata = vec![std::mem::zeroed(); count as usize]; |
There was a problem hiding this comment.
I am not sure about this one, I tried running this function on a machine with an a16 and count was a number that was much larger than the expected buffer size (468) but not a factor of std::mem::size_of::<nvmlVgpuMetadata_t>(); which is 212 according to rust analyzer.
|
I believe this is ready for review. I have tried to comment on any of the places where I wasn't quite sure about how something should be implemented. |
2 participants
This PR is an attempt to fully cover the vGPU APIs defined by nvml. To achieve this I've followed the patterns defined for
DeviceandVgpuTypeto cover all functions in theunwrapped_functions.txtfile that are namednvmlVgpu*The one exception here was
nvmlVgpuInstanceGetLicenseStatuswhich is deprecated and not documented at this time, if you'd like me to dig through older versions of the docs, I can find the documentation for how that API works.Please let me know if I've misunderstood any of the patterns I've tried to emulate and/or the API documentation, I would be happy to follow up with additional changes as needed.
Note: this is currently marked as a draft because I based these changes on #129 and will rebase once that merges or is declined