Skip to content

Commit e0eb3d4

Browse files
committed
fix(drivers): scope CDI validation to container drivers
Signed-off-by: Evan Lezar <elezar@nvidia.com>
1 parent e7c1d7f commit e0eb3d4

4 files changed

Lines changed: 44 additions & 31 deletions

File tree

‎crates/openshell-core/src/gpu.rs‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,22 @@ pub fn validate_cdi_device_names(devices: &[String], field: &str) -> Result<(),
342342
Ok(())
343343
}
344344

345+
/// Validate explicit CDI selectors and their GPU request requirements.
346+
///
347+
/// Docker and Podman use this helper; drivers with other identifier formats use
348+
/// [`validate_specific_gpu_device_request`] directly.
349+
///
350+
/// # Errors
351+
/// Returns an error for malformed CDI names or invalid GPU request requirements.
352+
pub fn validate_cdi_gpu_device_request(
353+
gpu: Option<&DriverGpuResourceRequirements>,
354+
devices: &[String],
355+
field: &str,
356+
) -> Result<(), String> {
357+
validate_cdi_device_names(devices, field)?;
358+
validate_specific_gpu_device_request(gpu, devices, field)
359+
}
360+
345361
/// Validate a compute-driver GPU request against driver-owned specific devices.
346362
///
347363
/// Drivers call this when a sandbox request combines portable GPU requirements
@@ -350,7 +366,7 @@ pub fn validate_cdi_device_names(devices: &[String], field: &str) -> Result<(),
350366
/// # Errors
351367
/// Returns an error when the sandbox GPU request is absent, when `gpu.count`
352368
/// is zero, when device IDs are duplicated, or when the effective GPU count
353-
/// does not equal the number of specific devices, or when CDI names are malformed.
369+
/// does not equal the number of specific devices.
354370
pub fn validate_specific_gpu_device_request(
355371
gpu: Option<&DriverGpuResourceRequirements>,
356372
specific_devices: &[String],
@@ -361,8 +377,6 @@ pub fn validate_specific_gpu_device_request(
361377
return Ok(());
362378
}
363379

364-
validate_cdi_device_names(specific_devices, driver_config_field)?;
365-
366380
let mut seen = HashSet::with_capacity(device_count);
367381
for device in specific_devices {
368382
if !seen.insert(device.as_str()) {
@@ -403,8 +417,7 @@ mod tests {
403417
"v/c=0",
404418
] {
405419
let devices = vec![device.to_string()];
406-
let gpu = DriverGpuResourceRequirements { count: None };
407-
validate_specific_gpu_device_request(Some(&gpu), &devices, "driver_config.cdi_devices")
420+
validate_cdi_device_names(&devices, "driver_config.cdi_devices")
408421
.unwrap_or_else(|error| panic!("{device:?}: {error}"));
409422
}
410423
}
@@ -441,13 +454,9 @@ mod tests {
441454
"vendor/gpu-=0",
442455
"vendor/gpu/other=0",
443456
] {
444-
let gpu = DriverGpuResourceRequirements { count: None };
445-
let error = validate_specific_gpu_device_request(
446-
Some(&gpu),
447-
&[device.to_string()],
448-
"driver_config.cdi_devices",
449-
)
450-
.unwrap_err();
457+
let error =
458+
validate_cdi_device_names(&[device.to_string()], "driver_config.cdi_devices")
459+
.unwrap_err();
451460
assert!(error.contains("CDI qualified names"), "{device:?}: {error}");
452461
}
453462
}
@@ -652,6 +661,18 @@ mod tests {
652661
);
653662
}
654663

664+
#[test]
665+
fn specific_gpu_device_request_preserves_non_cdi_identifiers() {
666+
let gpu = DriverGpuResourceRequirements { count: Some(1) };
667+
let devices = vec!["0000:2d:00.0".to_string()];
668+
validate_specific_gpu_device_request(Some(&gpu), &devices, "driver_config.gpu_device_ids")
669+
.expect("generic GPU request validation must accept PCI device identifiers");
670+
let error =
671+
validate_cdi_gpu_device_request(Some(&gpu), &devices, "driver_config.cdi_devices")
672+
.expect_err("CDI drivers must reject non-CDI identifiers");
673+
assert!(error.contains("CDI qualified names"));
674+
}
675+
655676
#[test]
656677
fn validate_specific_gpu_device_request_ignores_empty_devices() {
657678
validate_specific_gpu_device_request(None, &[], "driver_config.cdi_devices")

‎crates/openshell-driver-docker/src/lib.rs‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use openshell_core::driver_utils::{
3333
};
3434
use openshell_core::gpu::{
3535
CdiGpuDefaultSelector, CdiGpuInventory, CdiGpuSelectionError, driver_gpu_requirements,
36-
effective_driver_gpu_count, validate_cdi_device_names, validate_specific_gpu_device_request,
36+
effective_driver_gpu_count, validate_cdi_device_names, validate_cdi_gpu_device_request,
3737
};
3838
use openshell_core::progress::{
3939
PROGRESS_STEP_PULLING_IMAGE, PROGRESS_STEP_REQUESTING_SANDBOX, PROGRESS_STEP_STARTING_SANDBOX,
@@ -1126,7 +1126,7 @@ impl DockerComputeDriver {
11261126
}
11271127

11281128
if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() {
1129-
validate_specific_gpu_device_request(
1129+
validate_cdi_gpu_device_request(
11301130
gpu_requirements,
11311131
cdi_devices,
11321132
"driver_config.cdi_devices",
@@ -1309,7 +1309,7 @@ impl DockerComputeDriver {
13091309
) -> Result<Vec<String>, CdiGpuSelectionError>,
13101310
) -> Result<Option<Vec<String>>, Status> {
13111311
if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() {
1312-
validate_specific_gpu_device_request(
1312+
validate_cdi_gpu_device_request(
13131313
gpu_requirements,
13141314
cdi_devices,
13151315
"driver_config.cdi_devices",
@@ -5618,12 +5618,8 @@ fn build_container_create_body(
56185618
.as_ref()
56195619
.and_then(|spec| driver_gpu_requirements(spec.resource_requirements.as_ref()));
56205620
let cdi_devices = if let Some(cdi_devices) = driver_config.cdi_devices.as_ref() {
5621-
validate_specific_gpu_device_request(
5622-
gpu_requirements,
5623-
cdi_devices,
5624-
"driver_config.cdi_devices",
5625-
)
5626-
.map_err(Status::invalid_argument)?;
5621+
validate_cdi_gpu_device_request(gpu_requirements, cdi_devices, "driver_config.cdi_devices")
5622+
.map_err(Status::invalid_argument)?;
56275623
Some(cdi_devices.as_slice())
56285624
} else {
56295625
None

‎crates/openshell-driver-podman/src/container.rs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use openshell_core::ComputeDriverError;
88
use openshell_core::driver_mounts::SelinuxLabel;
99
use openshell_core::gpu::validate_cdi_device_names;
1010
#[cfg(test)]
11-
use openshell_core::gpu::{driver_gpu_requirements, validate_specific_gpu_device_request};
11+
use openshell_core::gpu::{driver_gpu_requirements, validate_cdi_gpu_device_request};
1212
use openshell_core::proto::compute::v1::{DriverSandbox, DriverSandboxTemplate};
1313
use openshell_core::proto_struct::deserialize_optional_non_empty_string_list;
1414
use openshell_core::{driver_mounts, proto_struct};
@@ -1040,12 +1040,8 @@ pub fn try_build_container_spec_with_token(
10401040
.as_ref()
10411041
.and_then(|spec| driver_gpu_requirements(spec.resource_requirements.as_ref()));
10421042
let cdi_devices = if let Some(cdi_devices) = driver_config.cdi_devices.as_ref() {
1043-
validate_specific_gpu_device_request(
1044-
gpu_requirements,
1045-
cdi_devices,
1046-
"driver_config.cdi_devices",
1047-
)
1048-
.map_err(ComputeDriverError::InvalidArgument)?;
1043+
validate_cdi_gpu_device_request(gpu_requirements, cdi_devices, "driver_config.cdi_devices")
1044+
.map_err(ComputeDriverError::InvalidArgument)?;
10491045
Some(cdi_devices.as_slice())
10501046
} else {
10511047
None

‎crates/openshell-driver-podman/src/driver.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use openshell_core::driver_utils::{
1818
};
1919
use openshell_core::gpu::{
2020
CdiGpuDefaultSelector, CdiGpuInventory, CdiGpuSelectionError, driver_gpu_requirements,
21-
effective_driver_gpu_count, validate_specific_gpu_device_request,
21+
effective_driver_gpu_count, validate_cdi_gpu_device_request,
2222
};
2323
use openshell_core::proto::compute::v1::{
2424
CpuResourceCapabilities, DriverSandbox, GetCapabilitiesResponse, GpuResourceCapabilities,
@@ -667,7 +667,7 @@ impl PodmanComputeDriver {
667667
let _ = effective_driver_gpu_count(gpu_requirements)
668668
.map_err(ComputeDriverError::InvalidArgument)?;
669669
if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() {
670-
validate_specific_gpu_device_request(
670+
validate_cdi_gpu_device_request(
671671
gpu_requirements,
672672
cdi_devices,
673673
"driver_config.cdi_devices",
@@ -693,7 +693,7 @@ impl PodmanComputeDriver {
693693
) -> Result<Vec<String>, CdiGpuSelectionError>,
694694
) -> Result<Option<Vec<String>>, ComputeDriverError> {
695695
if let Some(cdi_devices) = driver_config.cdi_devices.as_deref() {
696-
validate_specific_gpu_device_request(
696+
validate_cdi_gpu_device_request(
697697
gpu_requirements,
698698
cdi_devices,
699699
"driver_config.cdi_devices",

0 commit comments

Comments
 (0)