Skip to content

Commit 5bbb977

Browse files
committed
fix: Unblock virtio-blk discard by permitting fallocate in seccomp
- Refactored unmap/fstrim/discard logic from deprecated discard_support branch. Signed-off-by: LDagnachew <[email protected]>
1 parent ceeca6a commit 5bbb977

File tree

11 files changed

+229
-7
lines changed

11 files changed

+229
-7
lines changed

resources/seccomp/aarch64-unknown-linux-musl.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@
8585
"syscall": "lseek",
8686
"comment": "Used by the block device"
8787
},
88+
{
89+
"syscall": "fallocate",
90+
"comment": "Used by the block device for discard (hole punching)"
91+
},
8892
{
8993
"syscall": "mremap",
9094
"comment": "Used for re-allocating large memory regions, for example vectors"

resources/seccomp/x86_64-unknown-linux-musl.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,10 @@
8585
"syscall": "lseek",
8686
"comment": "Used by the block device"
8787
},
88+
{
89+
"syscall": "fallocate",
90+
"comment": "Used by the block device for discard (hole punching)"
91+
},
8892
{
8993
"syscall": "mremap",
9094
"comment": "Used for re-allocating large memory regions, for example vectors"

src/vmm/src/devices/virtio/block/virtio/device.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ use crate::devices::virtio::block::CacheType;
2727
use crate::devices::virtio::block::virtio::metrics::{BlockDeviceMetrics, BlockMetricsPerDevice};
2828
use crate::devices::virtio::device::{ActiveState, DeviceState, VirtioDevice};
2929
use crate::devices::virtio::generated::virtio_blk::{
30-
VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_BLK_ID_BYTES,
30+
VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_BLK_ID_BYTES, VIRTIO_BLK_F_DISCARD,
3131
};
3232
use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1;
3333
use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_BLOCK;
@@ -298,7 +298,7 @@ impl VirtioBlock {
298298
.map_err(VirtioBlockError::RateLimiter)?
299299
.unwrap_or_default();
300300

301-
let mut avail_features = (1u64 << VIRTIO_F_VERSION_1) | (1u64 << VIRTIO_RING_F_EVENT_IDX);
301+
let mut avail_features = (1u64 << VIRTIO_F_VERSION_1) | (1u64 << VIRTIO_RING_F_EVENT_IDX) | (1u64 << VIRTIO_BLK_F_DISCARD);
302302

303303
if config.cache_type == CacheType::Writeback {
304304
avail_features |= 1u64 << VIRTIO_BLK_F_FLUSH;

src/vmm/src/devices/virtio/block/virtio/io/async_io.rs

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ impl AsyncFileEngine {
110110
Ok(())
111111
}
112112

113-
#[cfg(test)]
114113
pub fn file(&self) -> &File {
115114
&self.file
116115
}
@@ -230,6 +229,27 @@ impl AsyncFileEngine {
230229
Ok(())
231230
}
232231

232+
pub fn push_discard(
233+
&mut self,
234+
offset: u64,
235+
len: u32,
236+
req: PendingRequest,
237+
) -> Result<(), RequestError<AsyncIoError>> {
238+
let wrapped_user_data = WrappedRequest::new(req);
239+
240+
self.ring
241+
.push(Operation::fallocate(
242+
0,
243+
len,
244+
offset,
245+
wrapped_user_data,
246+
))
247+
.map_err(|(io_uring_error, data)| RequestError {
248+
req: data.req,
249+
error: AsyncIoError::IoUring(io_uring_error),
250+
})
251+
}
252+
233253
fn do_pop(&mut self) -> Result<Option<Cqe<WrappedRequest>>, AsyncIoError> {
234254
self.ring.pop().map_err(AsyncIoError::IoUring)
235255
}

src/vmm/src/devices/virtio/block/virtio/io/mod.rs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,12 @@ pub mod sync_io;
66

77
use std::fmt::Debug;
88
use std::fs::File;
9+
use libc::{c_int, off64_t, FALLOC_FL_KEEP_SIZE, FALLOC_FL_PUNCH_HOLE};
10+
use std::os::unix::io::AsRawFd;
911

1012
pub use self::async_io::{AsyncFileEngine, AsyncIoError};
1113
pub use self::sync_io::{SyncFileEngine, SyncIoError};
14+
use crate::device_manager::mmio::MMIO_LEN;
1215
use crate::devices::virtio::block::virtio::PendingRequest;
1316
use crate::devices::virtio::block::virtio::device::FileEngineType;
1417
use crate::vstate::memory::{GuestAddress, GuestMemoryMmap};
@@ -75,7 +78,7 @@ impl FileEngine {
7578
Ok(())
7679
}
7780

78-
#[cfg(test)]
81+
7982
pub fn file(&self) -> &File {
8083
match self {
8184
FileEngine::Async(engine) => engine.file(),
@@ -172,6 +175,31 @@ impl FileEngine {
172175
FileEngine::Sync(engine) => engine.flush().map_err(BlockIoError::Sync),
173176
}
174177
}
178+
179+
pub fn discard(
180+
&mut self,
181+
offset: u64,
182+
count: u32,
183+
req: PendingRequest,
184+
) -> Result<FileEngineOk, RequestError<BlockIoError>> {
185+
186+
match self {
187+
FileEngine::Async(engine) => match engine.push_discard(offset, count, req) {
188+
Ok(_) => Ok(FileEngineOk::Submitted),
189+
Err(err) => Err(RequestError {
190+
req: err.req,
191+
error: BlockIoError::Async(err.error),
192+
}),
193+
},
194+
FileEngine::Sync(engine) => match engine.discard(offset,count) {
195+
Ok(count) => Ok(FileEngineOk::Executed(RequestOk { req, count })),
196+
Err(err) => Err(RequestError {
197+
req,
198+
error: BlockIoError::Sync(err),
199+
}),
200+
},
201+
}
202+
}
175203
}
176204

177205
#[cfg(test)]

src/vmm/src/devices/virtio/block/virtio/io/sync_io.rs

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
use std::fs::File;
55
use std::io::{Seek, SeekFrom, Write};
6+
use libc::{c_int, off64_t, FALLOC_FL_KEEP_SIZE, FALLOC_FL_PUNCH_HOLE};
7+
use std::os::unix::io::AsRawFd;
68

79
use vm_memory::{GuestMemoryError, ReadVolatile, WriteVolatile};
810

@@ -18,6 +20,8 @@ pub enum SyncIoError {
1820
SyncAll(std::io::Error),
1921
/// Transfer: {0}
2022
Transfer(GuestMemoryError),
23+
/// Discard: {0}
24+
Discard(std::io::Error)
2125
}
2226

2327
#[derive(Debug)]
@@ -33,7 +37,7 @@ impl SyncFileEngine {
3337
SyncFileEngine { file }
3438
}
3539

36-
#[cfg(test)]
40+
3741
pub fn file(&self) -> &File {
3842
&self.file
3943
}
@@ -81,4 +85,29 @@ impl SyncFileEngine {
8185
// Sync data out to physical media on host.
8286
self.file.sync_all().map_err(SyncIoError::SyncAll)
8387
}
88+
89+
pub fn discard(&mut self, offset: u64, len: u32) -> Result<u32, SyncIoError> {
90+
91+
unsafe {
92+
let ret = libc::fallocate(
93+
self.file.as_raw_fd(),
94+
libc::FALLOC_FL_PUNCH_HOLE | libc::FALLOC_FL_KEEP_SIZE,
95+
offset as i64,
96+
len as i64,
97+
);
98+
if ret != 0 {
99+
return Err(SyncIoError::Discard(std::io::Error::last_os_error()));
100+
}
101+
}
102+
Ok(len)
103+
}
104+
105+
pub fn fallocate(fd: c_int, mode: i32, offset: off64_t, len: off64_t) -> Result<(), std::io::Error> {
106+
let ret: i32 = unsafe { libc::fallocate(fd, mode, offset, len) };
107+
if ret == 0 {
108+
Ok(())
109+
} else {
110+
Err(std::io::Error::last_os_error())
111+
}
112+
}
84113
}

src/vmm/src/devices/virtio/block/virtio/metrics.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ pub struct BlockDeviceMetrics {
157157
pub invalid_reqs_count: SharedIncMetric,
158158
/// Number of flushes operation triggered on this block device.
159159
pub flush_count: SharedIncMetric,
160+
/// Number of discard operation triggered on this block device.
161+
pub discard_count: SharedIncMetric,
160162
/// Number of events triggered on the queue of this block device.
161163
pub queue_event_count: SharedIncMetric,
162164
/// Number of events ratelimiter-related.
@@ -210,6 +212,7 @@ impl BlockDeviceMetrics {
210212
self.invalid_reqs_count
211213
.add(other.invalid_reqs_count.fetch_diff());
212214
self.flush_count.add(other.flush_count.fetch_diff());
215+
self.discard_count.add(other.discard_count.fetch_diff());
213216
self.queue_event_count
214217
.add(other.queue_event_count.fetch_diff());
215218
self.rate_limiter_event_count

src/vmm/src/devices/virtio/block/virtio/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,12 @@ pub enum VirtioBlockError {
6363
RateLimiter(std::io::Error),
6464
/// Persistence error: {0}
6565
Persist(crate::devices::virtio::persist::PersistError),
66+
/// Sector overflow in discard segment
67+
SectorOverflow,
68+
/// Discard segment exceeds disk size
69+
BeyondDiskSize,
70+
/// Invalid flags in discard segment
71+
InvalidDiscardFlags,
72+
/// Invalid discard request (e.g., empty segments)
73+
InvalidDiscardRequest,
6674
}

0 commit comments

Comments
 (0)