-
Notifications
You must be signed in to change notification settings - Fork 182
storage: add usermode storvsc disk backend for OpenHCL #3193
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?
Changes from all commits
a630478
4b376dd
4204f9d
1aab8e9
7e4eecb
978f630
f5f430e
8ea22f7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -180,6 +180,9 @@ fn build_kernel_command_line( | |||||||
| "rdinit=/underhill-init", | ||||||||
| // Default to user-mode NVMe driver. | ||||||||
| "OPENHCL_NVME_VFIO=1", | ||||||||
| // TODO(juantian): TEMP -- enable usermode storvsc to validate via CI pipeline. | ||||||||
| // MUST be removed before merge. See PR description. | ||||||||
| "OPENHCL_STORVSC_USERMODE=1", | ||||||||
|
Comment on lines
+183
to
+185
|
||||||||
| // TODO(juantian): TEMP -- enable usermode storvsc to validate via CI pipeline. | |
| // MUST be removed before merge. See PR description. | |
| "OPENHCL_STORVSC_USERMODE=1", |
Copilot
AI
Apr 5, 2026
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.
This hard-codes OPENHCL_STORVSC_USERMODE=1 into the default kernel command line, which contradicts the PR’s “opt-in only / not enabled by default” requirement. Please remove this before merge (or gate it behind an explicit test-only feature/build flag) so production boots remain opt-in via environment.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,8 @@ | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| use super::LoadedVm; | ||||||||||||||||||||||||||||||
| use crate::nvme_manager::manager::NvmeDiskConfig; | ||||||||||||||||||||||||||||||
| use crate::storvsc_manager::StorvscDiskBounceConfig; | ||||||||||||||||||||||||||||||
| use crate::storvsc_manager::StorvscDiskConfig; | ||||||||||||||||||||||||||||||
| use crate::worker::NicConfig; | ||||||||||||||||||||||||||||||
| use anyhow::Context; | ||||||||||||||||||||||||||||||
| use cvm_tracing::CVM_ALLOWED; | ||||||||||||||||||||||||||||||
|
|
@@ -244,6 +246,7 @@ pub struct DeviceInterfaces { | |||||||||||||||||||||||||||||
| scsi_dvds: HashMap<StorageDevicePath, mesh::Sender<SimpleScsiDvdRequest>>, | ||||||||||||||||||||||||||||||
| scsi_request: HashMap<Guid, mesh::Sender<ScsiControllerRequest>>, | ||||||||||||||||||||||||||||||
| use_nvme_vfio: bool, | ||||||||||||||||||||||||||||||
| use_storvsc_usermode: bool, | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| impl Vtl2SettingsWorker { | ||||||||||||||||||||||||||||||
|
|
@@ -397,6 +400,7 @@ impl Vtl2SettingsWorker { | |||||||||||||||||||||||||||||
| &StorageContext { | ||||||||||||||||||||||||||||||
| uevent_listener, | ||||||||||||||||||||||||||||||
| use_nvme_vfio: self.interfaces.use_nvme_vfio, | ||||||||||||||||||||||||||||||
| use_storvsc_usermode: self.interfaces.use_storvsc_usermode, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| &disk, | ||||||||||||||||||||||||||||||
| false, | ||||||||||||||||||||||||||||||
|
|
@@ -415,6 +419,7 @@ impl Vtl2SettingsWorker { | |||||||||||||||||||||||||||||
| &StorageContext { | ||||||||||||||||||||||||||||||
| uevent_listener, | ||||||||||||||||||||||||||||||
| use_nvme_vfio: self.interfaces.use_nvme_vfio, | ||||||||||||||||||||||||||||||
| use_storvsc_usermode: self.interfaces.use_storvsc_usermode, | ||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||
| &disk, | ||||||||||||||||||||||||||||||
| false, | ||||||||||||||||||||||||||||||
|
|
@@ -978,6 +983,7 @@ async fn make_disk_type_from_physical_devices( | |||||||||||||||||||||||||||||
| struct StorageContext<'a> { | ||||||||||||||||||||||||||||||
| uevent_listener: &'a UeventListener, | ||||||||||||||||||||||||||||||
| use_nvme_vfio: bool, | ||||||||||||||||||||||||||||||
| use_storvsc_usermode: bool, | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| #[instrument(skip_all, fields(CVM_ALLOWED))] | ||||||||||||||||||||||||||||||
|
|
@@ -1021,6 +1027,27 @@ async fn make_disk_type_from_physical_device( | |||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // If storvsc usermode is enabled, route VScsi devices through StorvscDiskResolver | ||||||||||||||||||||||||||||||
| // instead of the kernel path. Early return -- no need to wait for kernel device. | ||||||||||||||||||||||||||||||
| if storage_context.use_storvsc_usermode | ||||||||||||||||||||||||||||||
| && matches!( | ||||||||||||||||||||||||||||||
| single_device.device_type, | ||||||||||||||||||||||||||||||
| underhill_config::DeviceType::VScsi | ||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| let lun = | ||||||||||||||||||||||||||||||
| u8::try_from(sub_device_path).map_err(|_| Error::StorageCannotFindVtl2Device { | ||||||||||||||||||||||||||||||
| device_type: single_device.device_type, | ||||||||||||||||||||||||||||||
| instance_id: controller_instance_id, | ||||||||||||||||||||||||||||||
| sub_device_path, | ||||||||||||||||||||||||||||||
| source: anyhow::anyhow!("sub_device_path {} exceeds u8 LUN range", sub_device_path), | ||||||||||||||||||||||||||||||
| })?; | ||||||||||||||||||||||||||||||
| return Ok(Resource::new(StorvscDiskConfig { | ||||||||||||||||||||||||||||||
| instance_guid: controller_instance_id, | ||||||||||||||||||||||||||||||
| lun, | ||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Wait for the device to arrive. | ||||||||||||||||||||||||||||||
| let devname = async { | ||||||||||||||||||||||||||||||
| let devname = ctx | ||||||||||||||||||||||||||||||
|
|
@@ -1211,11 +1238,35 @@ async fn make_ide_disk_config( | |||||||||||||||||||||||||||||
| Some(send), | ||||||||||||||||||||||||||||||
| )) | ||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||
| // When storvsc usermode is active and this IDE disk is backed by a | ||||||||||||||||||||||||||||||
| // VScsi controller, the IDE direct (port I/O) path needs a bounce | ||||||||||||||||||||||||||||||
| // wrapper because IDE CommandBuffer uses fake GPNs. The IDE accel | ||||||||||||||||||||||||||||||
| // (storvsp VMBus) path gets the normal GPA-direct disk. | ||||||||||||||||||||||||||||||
| let ide_direct = if storage_context.use_storvsc_usermode { | ||||||||||||||||||||||||||||||
| match &disk.physical_devices { | ||||||||||||||||||||||||||||||
| PhysicalDevices::Single { device } | ||||||||||||||||||||||||||||||
| if device.device_type == underhill_config::DeviceType::VScsi => | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| let lun = u8::try_from(device.sub_device_path).ok(); | ||||||||||||||||||||||||||||||
| lun.map(|lun| { | ||||||||||||||||||||||||||||||
| Resource::new(StorvscDiskBounceConfig { | ||||||||||||||||||||||||||||||
| instance_guid: device.vmbus_instance_id, | ||||||||||||||||||||||||||||||
| lun, | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||
|
Comment on lines
+1250
to
+1256
|
||||||||||||||||||||||||||||||
| let lun = u8::try_from(device.sub_device_path).ok(); | |
| lun.map(|lun| { | |
| Resource::new(StorvscDiskBounceConfig { | |
| instance_guid: device.vmbus_instance_id, | |
| lun, | |
| }) | |
| }) | |
| let lun = u8::try_from(device.sub_device_path).context( | |
| "VScsi IDE direct-path routing requires sub_device_path to fit in u8", | |
| )?; | |
| Some(Resource::new(StorvscDiskBounceConfig { | |
| instance_guid: device.vmbus_instance_id, | |
| lun, | |
| })) |
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.
This hard-codes
OPENHCL_STORVSC_USERMODE=1into the boot command line, which enables the feature by default and contradicts the PR’s stated opt-in behavior. Please remove this before merge (or gate it behind a test-only/dev-only configuration) so production images remain opt-in via environment only.