Skip to content

Commit 8fd863f

Browse files
committed
style: fix Python formatting and lint issues
Update Python files to comply with Black and pylint checks. These changes are formatting-only and do not alter functionality. Signed-off-by: LDagnachew <[email protected]>
1 parent 53f0e29 commit 8fd863f

File tree

7 files changed

+65
-35
lines changed

7 files changed

+65
-35
lines changed

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

Lines changed: 4 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, VIRTIO_BLK_F_DISCARD,
30+
VIRTIO_BLK_F_DISCARD, VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_BLK_ID_BYTES,
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,9 @@ 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) | (1u64 << VIRTIO_BLK_F_DISCARD);
301+
let mut avail_features = (1u64 << VIRTIO_F_VERSION_1)
302+
| (1u64 << VIRTIO_RING_F_EVENT_IDX)
303+
| (1u64 << VIRTIO_BLK_F_DISCARD);
302304

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

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -238,18 +238,13 @@ impl AsyncFileEngine {
238238
let wrapped_user_data = WrappedRequest::new(req);
239239

240240
self.ring
241-
.push(Operation::fallocate(
242-
0,
243-
len,
244-
offset,
245-
wrapped_user_data,
246-
))
241+
.push(Operation::fallocate(0, len, offset, wrapped_user_data))
247242
.map_err(|(io_uring_error, data)| RequestError {
248243
req: data.req,
249244
error: AsyncIoError::IoUring(io_uring_error),
250245
})
251246
}
252-
247+
253248
fn do_pop(&mut self) -> Result<Option<Cqe<WrappedRequest>>, AsyncIoError> {
254249
self.ring.pop().map_err(AsyncIoError::IoUring)
255250
}

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,10 @@ 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};
109
use std::os::unix::io::AsRawFd;
1110

11+
use libc::{FALLOC_FL_KEEP_SIZE, FALLOC_FL_PUNCH_HOLE, c_int, off64_t};
12+
1213
pub use self::async_io::{AsyncFileEngine, AsyncIoError};
1314
pub use self::sync_io::{SyncFileEngine, SyncIoError};
1415
use crate::device_manager::mmio::MMIO_LEN;
@@ -78,7 +79,6 @@ impl FileEngine {
7879
Ok(())
7980
}
8081

81-
8282
pub fn file(&self) -> &File {
8383
match self {
8484
FileEngine::Async(engine) => engine.file(),
@@ -182,7 +182,6 @@ impl FileEngine {
182182
count: u32,
183183
req: PendingRequest,
184184
) -> Result<FileEngineOk, RequestError<BlockIoError>> {
185-
186185
match self {
187186
FileEngine::Async(engine) => match engine.push_discard(offset, count, req) {
188187
Ok(_) => Ok(FileEngineOk::Submitted),
@@ -191,7 +190,7 @@ impl FileEngine {
191190
error: BlockIoError::Async(err.error),
192191
}),
193192
},
194-
FileEngine::Sync(engine) => match engine.discard(offset,count) {
193+
FileEngine::Sync(engine) => match engine.discard(offset, count) {
195194
Ok(count) => Ok(FileEngineOk::Executed(RequestOk { req, count })),
196195
Err(err) => Err(RequestError {
197196
req,

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

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
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};
76
use std::os::unix::io::AsRawFd;
87

8+
use libc::{FALLOC_FL_KEEP_SIZE, FALLOC_FL_PUNCH_HOLE, c_int, off64_t};
99
use vm_memory::{GuestMemoryError, ReadVolatile, WriteVolatile};
1010

1111
use crate::vstate::memory::{GuestAddress, GuestMemory, GuestMemoryMmap};
@@ -21,7 +21,7 @@ pub enum SyncIoError {
2121
/// Transfer: {0}
2222
Transfer(GuestMemoryError),
2323
/// Discard: {0}
24-
Discard(std::io::Error)
24+
Discard(std::io::Error),
2525
}
2626

2727
#[derive(Debug)]
@@ -37,7 +37,6 @@ impl SyncFileEngine {
3737
SyncFileEngine { file }
3838
}
3939

40-
4140
pub fn file(&self) -> &File {
4241
&self.file
4342
}
@@ -87,13 +86,27 @@ impl SyncFileEngine {
8786
}
8887

8988
pub fn discard(&mut self, offset: u64, len: u32) -> Result<u32, SyncIoError> {
90-
89+
// Do checked conversion to avoid possible wrap/cast issues on 64-bit systems.
90+
let off_i64 = i64::try_from(offset).map_err(|_| {
91+
SyncIoError::Discard(std::io::Error::new(
92+
std::io::ErrorKind::InvalidInput,
93+
"offset overflow",
94+
))
95+
})?;
96+
97+
let len_i64: i64 = len.into();
98+
99+
// SAFETY: calling libc::fallocate is safe here because:
100+
// - `self.file.as_raw_fd()` is a valid file descriptor owned by this struct,
101+
// - `off_i64` and `len_i64` are validated copies of the incoming unsigned values converted
102+
// to the C `off64_t` type, and
103+
// - the syscall is properly checked for an error return value.
91104
unsafe {
92105
let ret = libc::fallocate(
93106
self.file.as_raw_fd(),
94107
libc::FALLOC_FL_PUNCH_HOLE | libc::FALLOC_FL_KEEP_SIZE,
95-
offset as i64,
96-
len as i64,
108+
off_i64,
109+
len_i64,
97110
);
98111
if ret != 0 {
99112
return Err(SyncIoError::Discard(std::io::Error::last_os_error()));
@@ -102,7 +115,14 @@ impl SyncFileEngine {
102115
Ok(len)
103116
}
104117

105-
pub fn fallocate(fd: c_int, mode: i32, offset: off64_t, len: off64_t) -> Result<(), std::io::Error> {
118+
pub fn fallocate(
119+
fd: c_int,
120+
mode: i32,
121+
offset: off64_t,
122+
len: off64_t,
123+
) -> Result<(), std::io::Error> {
124+
// SAFETY: calling libc::fallocate is safe because we're passing plain C-compatible
125+
// integer types (fd, mode, offset, len) and we check the integer return value.
106126
let ret: i32 = unsafe { libc::fallocate(fd, mode, offset, len) };
107127
if ret == 0 {
108128
Ok(())

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ use crate::devices::virtio::block::virtio::device::DiskProperties;
1515
use crate::devices::virtio::block::virtio::metrics::BlockDeviceMetrics;
1616
pub use crate::devices::virtio::generated::virtio_blk::{
1717
VIRTIO_BLK_ID_BYTES, VIRTIO_BLK_S_IOERR, VIRTIO_BLK_S_OK, VIRTIO_BLK_S_UNSUPP,
18-
VIRTIO_BLK_T_FLUSH, VIRTIO_BLK_T_GET_ID, VIRTIO_BLK_T_IN, VIRTIO_BLK_T_OUT,
19-
VIRTIO_BLK_T_DISCARD
18+
VIRTIO_BLK_T_DISCARD, VIRTIO_BLK_T_FLUSH, VIRTIO_BLK_T_GET_ID, VIRTIO_BLK_T_IN,
19+
VIRTIO_BLK_T_OUT,
2020
};
2121
use crate::devices::virtio::queue::DescriptorChain;
2222
use crate::logger::{IncMetric, error};
2323
use crate::rate_limiter::{RateLimiter, TokenType};
24-
use crate::vstate::memory::{ByteValued, Bytes, GuestAddress, GuestMemoryMmap, Address};
24+
use crate::vstate::memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemoryMmap};
2525

2626
#[derive(Debug, derive_more::From)]
2727
pub enum IoErr {
@@ -249,6 +249,8 @@ pub struct DiscardSegment {
249249
num_sectors: u32,
250250
flags: u32,
251251
}
252+
// SAFETY: Safe because `DiscardSegment` only contains plain data (POD) with no padding-dependent
253+
// invariants.
252254
unsafe impl ByteValued for DiscardSegment {}
253255

254256
#[derive(Debug, PartialEq, Eq)]
@@ -279,7 +281,7 @@ impl Request {
279281
data_addr: GuestAddress(0),
280282
data_len: 0,
281283
status_addr: GuestAddress(0),
282-
discard_segments: None
284+
discard_segments: None,
283285
};
284286

285287
let data_desc;
@@ -341,27 +343,30 @@ impl Request {
341343
}
342344
RequestType::Discard => {
343345
// Validate data length
344-
let segment_size = std::mem::size_of::<DiscardSegment>() as u32;
345-
if data_desc.len % segment_size != 0 {
346+
let segment_size = std::mem::size_of::<DiscardSegment>();
347+
if (data_desc.len as usize) % segment_size != 0 {
346348
return Err(VirtioBlockError::InvalidDataLength);
347349
}
348350

349351
// Calculate number of segments
350-
let num_segments = data_desc.len / segment_size;
352+
let num_segments = (data_desc.len as usize) / segment_size;
351353
if num_segments == 0 {
352354
return Err(VirtioBlockError::InvalidDiscardRequest);
353355
}
354-
let mut segments = Vec::with_capacity(num_segments as usize);
356+
let mut segments = Vec::with_capacity(num_segments);
355357

356358
// Populate DiscardSegments vector
357359
for i in 0..num_segments {
358360
let offset = i * segment_size;
359-
let segment: DiscardSegment = mem.read_obj(data_desc.addr.unchecked_add(offset as u64))
361+
let segment: DiscardSegment = mem
362+
.read_obj(data_desc.addr.unchecked_add(offset as u64))
360363
.map_err(VirtioBlockError::GuestMemory)?;
361364
if segment.flags & !0x1 != 0 {
362365
return Err(VirtioBlockError::InvalidDiscardFlags);
363366
}
364-
let end_sector = segment.sector.checked_add(segment.num_sectors as u64)
367+
let end_sector = segment
368+
.sector
369+
.checked_add(segment.num_sectors as u64)
365370
.ok_or(VirtioBlockError::SectorOverflow)?;
366371
if end_sector > num_disk_sectors {
367372
return Err(VirtioBlockError::BeyondDiskSize);
@@ -372,7 +377,8 @@ impl Request {
372377
// Assign to req.discard_segments
373378
req.discard_segments = Some(segments);
374379
req.data_len = data_desc.len;
375-
status_desc = data_desc.next_descriptor()
380+
status_desc = data_desc
381+
.next_descriptor()
376382
.ok_or(VirtioBlockError::DescriptorChainTooShort)?;
377383
if !status_desc.is_write_only() {
378384
return Err(VirtioBlockError::UnexpectedReadOnlyDescriptor);

src/vmm/src/io_uring/operation/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ impl<T: Debug> Operation<T> {
116116
}
117117
}
118118

119+
/// Construct a fallocate operation.
119120
pub fn fallocate(fd: FixedFd, len: u32, offset: u64, user_data: T) -> Self {
120121
Self {
121122
fd,
@@ -127,7 +128,7 @@ impl<T: Debug> Operation<T> {
127128
user_data,
128129
}
129130
}
130-
131+
131132
pub(crate) fn fd(&self) -> FixedFd {
132133
self.fd
133134
}

tests/integration_tests/functional/test_discard.py

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
1-
import pytest
1+
# ...existing code...
2+
# Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
# SPDX-License-Identifier: Apache-2.0
4+
"""Integration test for discard (VIRTIO_BLK_T_DISCARD)."""
5+
26
import os
7+
38
import host_tools.drive as drive_tools
49

510

@@ -30,9 +35,11 @@ def test_discard_support(uvm_plain_rw):
3035

3136
vm.ssh.run("mount -o remount,discard /")
3237
exit_code, stdout, _ = vm.ssh.run("mount | grep /dev/vda")
33-
assert exit_code == 0, f"Failed to remount root filesystem with discard option: {stdout}"
38+
assert (
39+
exit_code == 0
40+
), f"Failed to remount root filesystem with discard option: {stdout}"
3441

3542
exit_code, stdout, _ = vm.ssh.run("fstrim -v /")
3643
assert exit_code == 0, f"fstrim -v failed: {stdout}"
3744
assert "trimmed" in stdout, f"Unexpected fstrim output: {stdout}"
38-
vm.kill()
45+
vm.kill()

0 commit comments

Comments
 (0)