Skip to content

Commit 59e809a

Browse files
committed
devices: block: activate through an activation event
Postpone block external events registration to device activation time. This makes the block device unaware of external events prior to its activation. During creation register a dedicated activation event which will notify the device when it's time to register the other external events sources. This activation event is unregistered after successful device activation. Signed-off-by: Adrian Catangiu <acatan@amazon.com>
1 parent c2ed89e commit 59e809a

File tree

5 files changed

+187
-51
lines changed

5 files changed

+187
-51
lines changed

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

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use virtio_gen::virtio_blk::*;
2121
use vm_memory::{Bytes, GuestMemoryMmap};
2222

2323
use super::{
24-
super::{ActivateResult, Queue, VirtioDevice, TYPE_BLOCK, VIRTIO_MMIO_INT_VRING},
24+
super::{ActivateResult, DeviceState, Queue, VirtioDevice, TYPE_BLOCK, VIRTIO_MMIO_INT_VRING},
2525
request::*,
2626
Error, CONFIG_SPACE_SIZE, QUEUE_SIZES, SECTOR_SHIFT, SECTOR_SIZE,
2727
};
@@ -88,15 +88,15 @@ pub struct Block {
8888
avail_features: u64,
8989
acked_features: u64,
9090
config_space: Vec<u8>,
91+
pub(crate) activate_evt: EventFd,
9192

9293
// Transport related fields.
9394
queues: Vec<Queue>,
9495
interrupt_status: Arc<AtomicUsize>,
9596
interrupt_evt: EventFd,
9697
pub(crate) queue_evts: [EventFd; 1],
97-
mem: GuestMemoryMmap,
9898

99-
device_activated: bool,
99+
pub(crate) device_state: DeviceState,
100100

101101
// Implementation specific fields.
102102
pub(crate) rate_limiter: RateLimiter,
@@ -107,7 +107,6 @@ impl Block {
107107
///
108108
/// The given file must be seekable and sizable.
109109
pub fn new(
110-
mem: GuestMemoryMmap,
111110
mut disk_image: File,
112111
is_disk_read_only: bool,
113112
rate_limiter: RateLimiter,
@@ -132,12 +131,12 @@ impl Block {
132131
acked_features: 0u64,
133132
config_space: build_config_space(disk_size),
134133
rate_limiter,
135-
mem,
136134
interrupt_status: Arc::new(AtomicUsize::new(0)),
137135
interrupt_evt: EventFd::new(libc::EFD_NONBLOCK)?,
138136
queue_evts,
139137
queues,
140-
device_activated: false,
138+
device_state: DeviceState::Inactive,
139+
activate_evt: EventFd::new(libc::EFD_NONBLOCK)?,
141140
})
142141
}
143142

@@ -161,11 +160,16 @@ impl Block {
161160
}
162161

163162
pub(crate) fn process_queue(&mut self, queue_index: usize) -> bool {
163+
let mem = match self.device_state {
164+
DeviceState::Activated(ref mem) => mem,
165+
// This should never happen, it's been already validated in the event handler.
166+
DeviceState::Inactive => return false,
167+
};
164168
let queue = &mut self.queues[queue_index];
165169
let mut used_any = false;
166-
while let Some(head) = queue.pop(&self.mem) {
170+
while let Some(head) = queue.pop(mem) {
167171
let len;
168-
match Request::parse(&head, &self.mem) {
172+
match Request::parse(&head, mem) {
169173
Ok(request) => {
170174
// If limiter.consume() fails it means there is no more TokenType::Ops
171175
// budget and rate limiting is in effect.
@@ -196,7 +200,7 @@ impl Block {
196200
let status = match request.execute(
197201
&mut self.disk_image,
198202
self.disk_nsectors,
199-
&self.mem,
203+
mem,
200204
&self.disk_image_id,
201205
) {
202206
Ok(l) => {
@@ -212,15 +216,15 @@ impl Block {
212216
};
213217
// We use unwrap because the request parsing process already checked that the
214218
// status_addr was valid.
215-
self.mem.write_obj(status, request.status_addr).unwrap();
219+
mem.write_obj(status, request.status_addr).unwrap();
216220
}
217221
Err(e) => {
218222
error!("Failed to parse available descriptor chain: {:?}", e);
219223
METRICS.block.execute_fails.inc();
220224
len = 0;
221225
}
222226
}
223-
queue.add_used(&self.mem, head.index, len);
227+
queue.add_used(mem, head.index, len);
224228
used_any = true;
225229
}
226230

@@ -314,17 +318,24 @@ impl VirtioDevice for Block {
314318
}
315319

316320
fn is_activated(&self) -> bool {
317-
self.device_activated
321+
match self.device_state {
322+
DeviceState::Inactive => false,
323+
DeviceState::Activated(_) => true,
324+
}
318325
}
319326

320-
fn activate(&mut self, _: GuestMemoryMmap) -> ActivateResult {
321-
self.device_activated = true;
327+
fn activate(&mut self, mem: GuestMemoryMmap) -> ActivateResult {
328+
if self.activate_evt.write(1).is_err() {
329+
error!("Block: Cannot write to activate_evt");
330+
return Err(super::super::ActivateError::BadActivate);
331+
}
332+
self.device_state = DeviceState::Activated(mem);
322333
Ok(())
323334
}
324335
}
325336

326337
#[cfg(test)]
327-
mod tests {
338+
pub(crate) mod tests {
328339
use std::fs::metadata;
329340
use std::os::unix::io::AsRawFd;
330341
use std::thread;
@@ -348,7 +359,7 @@ mod tests {
348359
}
349360

350361
impl Block {
351-
fn set_queue(&mut self, idx: usize, q: Queue) {
362+
pub(crate) fn set_queue(&mut self, idx: usize, q: Queue) {
352363
self.queues[idx] = q;
353364
}
354365

@@ -362,7 +373,7 @@ mod tests {
362373
}
363374

364375
/// Create a default Block instance to be used in tests.
365-
fn default_block() -> Block {
376+
pub fn default_block() -> Block {
366377
// Create backing file.
367378
let f = TempFile::new().unwrap();
368379
let block_file = f.into_file();
@@ -371,12 +382,14 @@ mod tests {
371382
// Rate limiting is enabled but with a high operation rate (10 million ops/s).
372383
let rate_limiter = RateLimiter::new(0, None, 0, 100_000, None, 10).unwrap();
373384

374-
let mem = GuestMemoryMmap::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap();
385+
Block::new(block_file, true, rate_limiter).unwrap()
386+
}
375387

376-
Block::new(mem, block_file, true, rate_limiter).unwrap()
388+
pub fn default_mem() -> GuestMemoryMmap {
389+
GuestMemoryMmap::from_ranges(&[(GuestAddress(0), 0x10000)]).unwrap()
377390
}
378391

379-
fn initialize_virtqueue(vq: &VirtQueue) {
392+
pub fn initialize_virtqueue(vq: &VirtQueue) {
380393
let request_type_desc: usize = 0;
381394
let data_desc: usize = 1;
382395
let status_desc: usize = 2;
@@ -492,7 +505,7 @@ mod tests {
492505
#[test]
493506
fn test_invalid_request() {
494507
let mut block = default_block();
495-
let mem = block.mem.clone();
508+
let mem = default_mem();
496509
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
497510
block.set_queue(0, vq.create_queue());
498511
block.activate(mem.clone()).unwrap();
@@ -517,7 +530,7 @@ mod tests {
517530
#[test]
518531
fn test_request_execute_failures() {
519532
let mut block = default_block();
520-
let mem = block.mem.clone();
533+
let mem = default_mem();
521534
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
522535
block.set_queue(0, vq.create_queue());
523536
block.activate(mem.clone()).unwrap();
@@ -575,7 +588,7 @@ mod tests {
575588
#[test]
576589
fn test_unsupported_request_type() {
577590
let mut block = default_block();
578-
let mem = block.mem.clone();
591+
let mem = default_mem();
579592
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
580593
block.set_queue(0, vq.create_queue());
581594
block.activate(mem.clone()).unwrap();
@@ -605,7 +618,7 @@ mod tests {
605618
#[test]
606619
fn test_read_write() {
607620
let mut block = default_block();
608-
let mem = block.mem.clone();
621+
let mem = default_mem();
609622
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
610623
block.set_queue(0, vq.create_queue());
611624
block.activate(mem.clone()).unwrap();
@@ -664,7 +677,7 @@ mod tests {
664677
#[test]
665678
fn test_flush() {
666679
let mut block = default_block();
667-
let mem = block.mem.clone();
680+
let mem = default_mem();
668681
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
669682
block.set_queue(0, vq.create_queue());
670683
block.activate(mem.clone()).unwrap();
@@ -707,7 +720,7 @@ mod tests {
707720
#[test]
708721
fn test_get_device_id() {
709722
let mut block = default_block();
710-
let mem = block.mem.clone();
723+
let mem = default_mem();
711724
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
712725
block.set_queue(0, vq.create_queue());
713726
block.activate(mem.clone()).unwrap();
@@ -773,7 +786,7 @@ mod tests {
773786
#[test]
774787
fn test_bandwidth_rate_limiter() {
775788
let mut block = default_block();
776-
let mem = block.mem.clone();
789+
let mem = default_mem();
777790
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
778791
block.set_queue(0, vq.create_queue());
779792
block.activate(mem.clone()).unwrap();
@@ -838,7 +851,7 @@ mod tests {
838851
#[test]
839852
fn test_ops_rate_limiter() {
840853
let mut block = default_block();
841-
let mem = block.mem.clone();
854+
let mem = default_mem();
842855
let vq = VirtQueue::new(GuestAddress(0), &mem, 16);
843856
block.set_queue(0, vq.create_queue());
844857
block.activate(mem.clone()).unwrap();

0 commit comments

Comments
 (0)