mirror of
https://github.com/cloud-hypervisor/cloud-hypervisor.git
synced 2026-08-05 02:19:16 +00:00
block: Restrict DISCARD to explicit sparse=true
PR #7852 fixed the missing VirtioBlockConfig fields but did not change the feature advertisement logic. The condition `sparse || disk_image.supports_zero_flag()` causes qcow2 to advertise DISCARD even with sparse=false, because qcow2 can mark clusters as zero (supports_zero_flag() returns true). Windows viostor BSODs (DRIVER_IRQL_NOT_LESS_OR_EQUAL) when DISCARD is advertised on qcow2 backends, making sparse=off ineffective as a workaround for qcow2 images. Restrict DISCARD to explicit sparse=true only. WRITE_ZEROES remains available for all sparse-capable backends. Fixes #7849 Signed-off-by: CMGS <ilskdw@gmail.com>
This commit is contained in:
@@ -4564,7 +4564,6 @@ mod common_parallel {
|
||||
#[test]
|
||||
fn test_virtio_block_sparse_off_qcow2() {
|
||||
const TEST_DISK_SIZE: &str = "2G";
|
||||
const CLUSTER_SIZE_BYTES: u64 = 64 * 1024;
|
||||
|
||||
let disk_config = UbuntuDiskConfig::new(JAMMY_IMAGE_NAME.to_string());
|
||||
let guest = Guest::new(Box::new(disk_config));
|
||||
@@ -4618,37 +4617,36 @@ mod common_parallel {
|
||||
1
|
||||
);
|
||||
|
||||
let mut current_offset_kb = 1024;
|
||||
|
||||
for &size_kb in BLOCK_DISCARD_TEST_SIZES_KB.iter() {
|
||||
guest
|
||||
.ssh_command(&format!(
|
||||
"sudo dd if=/dev/urandom of=/dev/vdc bs=1K count={size_kb} seek={current_offset_kb} oflag=direct"
|
||||
))
|
||||
.unwrap();
|
||||
|
||||
guest.ssh_command("sync").unwrap();
|
||||
|
||||
guest
|
||||
.ssh_command(&format!(
|
||||
"sudo blkdiscard -o {} -l {} /dev/vdc",
|
||||
current_offset_kb * 1024,
|
||||
size_kb * 1024
|
||||
))
|
||||
.unwrap();
|
||||
|
||||
guest.ssh_command("sync").unwrap();
|
||||
|
||||
// Verify VM sees zeros in discarded region
|
||||
assert_guest_disk_region_is_zero(
|
||||
&guest,
|
||||
"/dev/vdc",
|
||||
current_offset_kb * 1024,
|
||||
size_kb * 1024,
|
||||
// With sparse=off, DISCARD should NOT be advertised.
|
||||
// blkdiscard is expected to fail.
|
||||
let discard_result =
|
||||
guest.ssh_command("sudo blkdiscard -o 1048576 -l 1048576 /dev/vdc 2>&1; echo $?");
|
||||
let exit_code = discard_result
|
||||
.unwrap()
|
||||
.trim()
|
||||
.lines()
|
||||
.last()
|
||||
.unwrap_or("1")
|
||||
.parse::<u32>()
|
||||
.unwrap_or(1);
|
||||
assert_ne!(
|
||||
exit_code, 0,
|
||||
"blkdiscard should fail with sparse=off (DISCARD not advertised)"
|
||||
);
|
||||
|
||||
current_offset_kb += size_kb + 64;
|
||||
}
|
||||
// WRITE_ZEROES should still work via blkdiscard --zeroout
|
||||
guest
|
||||
.ssh_command(
|
||||
"sudo dd if=/dev/urandom of=/dev/vdc bs=1K count=64 seek=1024 oflag=direct",
|
||||
)
|
||||
.unwrap();
|
||||
guest.ssh_command("sync").unwrap();
|
||||
guest
|
||||
.ssh_command("sudo blkdiscard -z -o 1048576 -l 65536 /dev/vdc")
|
||||
.unwrap();
|
||||
guest.ssh_command("sync").unwrap();
|
||||
|
||||
assert_guest_disk_region_is_zero(&guest, "/dev/vdc", 1048576, 65536);
|
||||
});
|
||||
|
||||
kill_child(&mut child);
|
||||
@@ -4659,9 +4657,10 @@ mod common_parallel {
|
||||
|
||||
handle_child_output(r, &output);
|
||||
|
||||
// WRITE_ZEROES should still produce zero-flagged regions
|
||||
assert!(
|
||||
zero_regions_after > zero_regions_before,
|
||||
"Expected zero-flagged regions to increase with sparse=off: before={zero_regions_before}, after={zero_regions_after}"
|
||||
"Expected zero-flagged regions to increase via WRITE_ZEROES: before={zero_regions_before}, after={zero_regions_after}"
|
||||
);
|
||||
|
||||
disk_check_consistency(test_disk_path, None);
|
||||
|
||||
@@ -778,13 +778,14 @@ impl Block {
|
||||
| (1u64 << VIRTIO_RING_F_INDIRECT_DESC);
|
||||
|
||||
// When backend supports sparse operations:
|
||||
// - Always advertise WRITE_ZEROES
|
||||
// - Advertise DISCARD only if sparse=true OR format supports marking
|
||||
// clusters as zero without deallocating
|
||||
// - Always advertise WRITE_ZEROES (safe for all drivers)
|
||||
// - Advertise DISCARD only when sparse=true, since DISCARD
|
||||
// deallocates space via punch_hole and should require
|
||||
// explicit user opt in.
|
||||
let mut discard_supported = false;
|
||||
if disk_image.supports_sparse_operations() {
|
||||
avail_features |= 1u64 << VIRTIO_BLK_F_WRITE_ZEROES;
|
||||
if sparse || disk_image.supports_zero_flag() {
|
||||
if sparse {
|
||||
avail_features |= 1u64 << VIRTIO_BLK_F_DISCARD;
|
||||
discard_supported = true;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user