Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/devices/src/virtio/block/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,19 @@ impl Block {
.write(!is_disk_read_only)
.open(PathBuf::from(&disk_image_path))?;

let disk_image_id = DiskProperties::build_disk_image_id(&disk_image);
// Use the caller-supplied `id` as the virtio-blk disk image id so
// it surfaces in the guest at `/sys/block/<dev>/serial` and (when
// udev is available) `/dev/disk/by-id/virtio-<id>`. Falls back to
// the rdev+inode-derived default if `id` is empty.
let disk_image_id = if id.is_empty() {
DiskProperties::build_disk_image_id(&disk_image)
} else {
let mut padded = vec![0u8; VIRTIO_BLK_ID_BYTES as usize];
let bytes = id.as_bytes();
let n = cmp::min(bytes.len(), padded.len());
padded[..n].copy_from_slice(&bytes[..n]);
padded
};

let file_opts = StorageOpenOptions::new()
.write(!is_disk_read_only)
Expand Down
130 changes: 124 additions & 6 deletions src/krun/src/api/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,30 @@ impl VmBuilder {

/// Configure block devices.
///
/// Can be called multiple times to add multiple devices.
/// Can be called multiple times to add multiple devices. Devices receive
/// deterministic guest names by attach order (`/dev/vda`, `/dev/vdb`,
/// ...). For stable addressing across reorderings, set a custom `id()` —
/// the guest can then reach the disk via `/dev/disk/by-id/virtio-<id>`.
///
/// # Example
/// # Examples
///
/// Single rootfs disk:
///
/// ```rust,no_run
/// # use msb_krun::VmBuilder;
/// VmBuilder::new()
/// .disk(|d| d.path("/path/to/disk.img").read_only(true));
/// ```
///
/// Rootfs plus data and cache volumes with stable ids:
///
/// ```rust,no_run
/// # use msb_krun::{VmBuilder, DiskImageFormat};
/// VmBuilder::new()
/// .disk(|d| d.path("/img/root.raw"))
/// .disk(|d| d.path("/img/data.qcow2").format(DiskImageFormat::Qcow2).id("data"))
/// .disk(|d| d.path("/img/cache.raw").id("cache").read_only(true));
/// ```
#[cfg(feature = "blk")]
pub fn disk(mut self, f: impl FnOnce(DiskBuilder) -> DiskBuilder) -> Self {
let new_disk = f(DiskBuilder::new()).finalize();
Expand Down Expand Up @@ -437,17 +452,22 @@ impl VmBuilder {
// Apply block device configuration
#[cfg(feature = "blk")]
for (i, config) in self.disk.configs.into_iter().enumerate() {
let block_id = format!("vd{}", (b'a' + i as u8) as char);
let block_id = config
.id
.clone()
.unwrap_or_else(|| format!("vd{}", vd_suffix(i)));
let image_type: ImageType = config.format.into();
let cache_type: CacheType = config.cache.into();
let sync_mode: devices::virtio::block::SyncMode = config.sync.into();

let blk_config = BlockDeviceConfig {
block_id,
cache_type: CacheType::Writeback,
cache_type,
disk_image_path: config.path.to_string_lossy().to_string(),
disk_image_format: image_type,
is_disk_read_only: config.read_only,
direct_io: false,
sync_mode: devices::virtio::block::SyncMode::default(),
direct_io: config.direct_io,
sync_mode,
};

vmr.add_block_device(blk_config)
Expand Down Expand Up @@ -543,6 +563,27 @@ fn generate_mac(index: usize) -> [u8; 6] {
]
}

/// Generate a virtio-blk device suffix matching the Linux kernel's
/// `disk_name()` bijective base-26 scheme from `block/genhd.c`:
/// `0→"a"`, `25→"z"`, `26→"aa"`, `27→"ab"`, `701→"zz"`, `702→"aaa"`.
///
/// The naïve `(b'a' + i) as char` formula rolls over past `z` into
/// invalid characters (`vd{`, `vd|`, ...), so this helper is required
/// once we support more than 26 disks.
#[cfg(feature = "blk")]
fn vd_suffix(mut index: usize) -> String {
let mut buf = Vec::with_capacity(4);
loop {
buf.push(b'a' + (index % 26) as u8);
if index < 26 {
break;
}
index = index / 26 - 1;
}
buf.reverse();
String::from_utf8(buf).expect("ASCII a-z only")
}

fn map_vm_config_error(machine: &MachineBuilder, err: VmConfigError) -> Error {
match err {
VmConfigError::InvalidVcpuCount => {
Expand Down Expand Up @@ -577,4 +618,81 @@ mod tests {
other => panic!("unexpected error: {other:?}"),
}
}

#[cfg(feature = "blk")]
#[test]
fn vd_suffix_matches_kernel_scheme() {
assert_eq!(vd_suffix(0), "a");
assert_eq!(vd_suffix(25), "z");
assert_eq!(vd_suffix(26), "aa");
assert_eq!(vd_suffix(27), "ab");
assert_eq!(vd_suffix(51), "az");
assert_eq!(vd_suffix(52), "ba");
assert_eq!(vd_suffix(701), "zz");
assert_eq!(vd_suffix(702), "aaa");
}

#[cfg(feature = "blk")]
#[test]
fn disk_builder_preserves_insertion_order() {
use crate::api::builders::DiskImageFormat;

let builder = VmBuilder::new()
.disk(|d| d.path("/a.raw"))
.disk(|d| d.path("/b.qcow2").format(DiskImageFormat::Qcow2))
.disk(|d| {
d.path("/c.vmdk")
.format(DiskImageFormat::Vmdk)
.read_only(true)
});

let paths: Vec<_> = builder
.disk
.configs
.iter()
.map(|c| c.path.to_string_lossy().into_owned())
.collect();
assert_eq!(paths, vec!["/a.raw", "/b.qcow2", "/c.vmdk"]);
assert_eq!(builder.disk.configs[1].format, DiskImageFormat::Qcow2);
assert!(builder.disk.configs[2].read_only);
}

#[cfg(feature = "blk")]
#[test]
fn disk_builder_auto_id_then_custom() {
let builder = VmBuilder::new()
.disk(|d| d.path("/a.raw"))
.disk(|d| d.path("/b.raw").id("data"))
.disk(|d| d.path("/c.raw"));

assert!(builder.disk.configs[0].id.is_none());
assert_eq!(builder.disk.configs[1].id.as_deref(), Some("data"));
assert!(builder.disk.configs[2].id.is_none());
}

#[cfg(feature = "blk")]
#[test]
fn disk_builder_per_disk_settings_dont_leak() {
use crate::api::builders::{CacheMode, SyncMode};

let builder = VmBuilder::new()
.disk(|d| {
d.path("/a.raw")
.read_only(true)
.cache(CacheMode::Unsafe)
.direct_io(true)
.sync(SyncMode::None)
})
.disk(|d| d.path("/b.raw"));

assert!(builder.disk.configs[0].read_only);
assert_eq!(builder.disk.configs[0].cache, CacheMode::Unsafe);
assert!(builder.disk.configs[0].direct_io);
assert_eq!(builder.disk.configs[0].sync, SyncMode::None);

assert!(!builder.disk.configs[1].read_only);
assert_eq!(builder.disk.configs[1].cache, CacheMode::Writeback);
assert!(!builder.disk.configs[1].direct_io);
assert_eq!(builder.disk.configs[1].sync, SyncMode::Full);
}
}
100 changes: 100 additions & 0 deletions src/krun/src/api/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,30 @@ pub enum DiskImageFormat {
Vmdk,
}

/// Host-side cache behavior for a block device.
///
/// Mirrors libkrun's internal `CacheType`. `Writeback` honors guest flush
/// requests via `fsync`; `Unsafe` advertises flush but makes it a noop.
#[cfg(feature = "blk")]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum CacheMode {
Writeback,
Unsafe,
}

/// Host-side sync behavior for a block device.
///
/// `None` skips all host sync (fastest, data-loss risk). `Relaxed` honors
/// `VIRTIO_BLK_F_FLUSH` but relaxes hardware sync. `Full` forces strict
/// hardware flush.
#[cfg(feature = "blk")]
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum SyncMode {
None,
Relaxed,
Full,
}

/// Builder for block device configuration.
///
/// # Example
Expand All @@ -273,17 +297,27 @@ pub enum DiskImageFormat {
pub struct DiskBuilder {
pub(crate) configs: Vec<DiskConfig>,
current_path: Option<PathBuf>,
current_id: Option<String>,
current_read_only: bool,
current_format: DiskImageFormat,
current_cache: CacheMode,
current_direct_io: bool,
current_sync: SyncMode,
}

/// Configuration for a single block device.
#[cfg(feature = "blk")]
#[derive(Debug, Clone)]
pub struct DiskConfig {
pub path: PathBuf,
/// Caller-chosen block id. When `None`, the builder auto-assigns
/// `vd<suffix>` using the kernel's bijective base-26 scheme.
pub id: Option<String>,
pub read_only: bool,
pub format: DiskImageFormat,
pub cache: CacheMode,
pub direct_io: bool,
pub sync: SyncMode,
}

//--------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -722,8 +756,12 @@ impl DiskBuilder {
Self {
configs: Vec::new(),
current_path: None,
current_id: None,
current_read_only: false,
current_format: DiskImageFormat::Raw,
current_cache: CacheMode::Writeback,
current_direct_io: false,
current_sync: SyncMode::Full,
}
}

Expand All @@ -733,17 +771,36 @@ impl DiskBuilder {
self
}

/// Set a stable block id for the current disk.
///
/// When unset, the builder auto-assigns `vd<suffix>` by insertion order
/// (matching the kernel's bijective base-26 scheme: `vda`..`vdz`,
/// `vdaa`..`vdzz`, `vdaaa`...). Setting a custom id lets the guest
/// address the device via `/dev/disk/by-id/virtio-<id>` independent of
/// attach order.
pub fn id(mut self, id: impl Into<String>) -> Self {
self.current_id = Some(id.into());
self
}

/// Set the path for a block device.
pub fn path(mut self, path: impl AsRef<Path>) -> Self {
// Finalize any pending config
if let Some(pending_path) = self.current_path.take() {
self.configs.push(DiskConfig {
path: pending_path,
id: self.current_id.take(),
read_only: self.current_read_only,
format: self.current_format,
cache: self.current_cache,
direct_io: self.current_direct_io,
sync: self.current_sync,
});
self.current_read_only = false;
self.current_format = DiskImageFormat::Raw;
self.current_cache = CacheMode::Writeback;
self.current_direct_io = false;
self.current_sync = SyncMode::Full;
}

self.current_path = Some(path.as_ref().to_path_buf());
Expand All @@ -756,13 +813,35 @@ impl DiskBuilder {
self
}

/// Set the host-side cache behavior for the current disk.
pub fn cache(mut self, mode: CacheMode) -> Self {
self.current_cache = mode;
self
}

/// Enable or disable `O_DIRECT` on the host backing file for the current disk.
pub fn direct_io(mut self, enabled: bool) -> Self {
self.current_direct_io = enabled;
self
}

/// Set the host-side sync behavior for the current disk.
pub fn sync(mut self, mode: SyncMode) -> Self {
self.current_sync = mode;
self
}

/// Finalize the builder (called internally).
pub(crate) fn finalize(mut self) -> Self {
if let Some(path) = self.current_path.take() {
self.configs.push(DiskConfig {
path,
id: self.current_id.take(),
read_only: self.current_read_only,
format: self.current_format,
cache: self.current_cache,
direct_io: self.current_direct_io,
sync: self.current_sync,
});
}
self
Expand Down Expand Up @@ -790,3 +869,24 @@ impl From<DiskImageFormat> for devices::virtio::block::ImageType {
}
}
}

#[cfg(feature = "blk")]
impl From<CacheMode> for devices::virtio::CacheType {
fn from(mode: CacheMode) -> Self {
match mode {
CacheMode::Writeback => devices::virtio::CacheType::Writeback,
CacheMode::Unsafe => devices::virtio::CacheType::Unsafe,
}
}
}

#[cfg(feature = "blk")]
impl From<SyncMode> for devices::virtio::block::SyncMode {
fn from(mode: SyncMode) -> Self {
match mode {
SyncMode::None => devices::virtio::block::SyncMode::None,
SyncMode::Relaxed => devices::virtio::block::SyncMode::Relaxed,
SyncMode::Full => devices::virtio::block::SyncMode::Full,
}
}
}
4 changes: 4 additions & 0 deletions src/krun/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,15 @@ pub mod backends;

pub use api::builder::VmBuilder;
#[cfg(feature = "blk")]
pub use api::builders::CacheMode;
#[cfg(feature = "blk")]
pub use api::builders::DiskBuilder;
#[cfg(feature = "blk")]
pub use api::builders::DiskImageFormat;
#[cfg(feature = "net")]
pub use api::builders::NetBuilder;
#[cfg(feature = "blk")]
pub use api::builders::SyncMode;
pub use api::builders::{ConsoleBuilder, ExecBuilder, FsBuilder, KernelBuilder, MachineBuilder};
pub use api::error::{BuildError, ConfigError, Error, Result, RuntimeError};
pub use api::exit_handle::ExitHandle;
Expand Down
Loading