diff --git a/bin/propolis-server/src/lib/stats/virtual_disk.rs b/bin/propolis-server/src/lib/stats/virtual_disk.rs index 4a39cfde0..522b72231 100644 --- a/bin/propolis-server/src/lib/stats/virtual_disk.rs +++ b/bin/propolis-server/src/lib/stats/virtual_disk.rs @@ -87,7 +87,7 @@ impl VirtualDiskStats { fn on_read_completion( &mut self, result: block::Result, - len: usize, + len: u64, duration: Duration, ) { let index = match result { @@ -95,9 +95,9 @@ impl VirtualDiskStats { let _ = self.io_latency[READ_INDEX] .datum .sample(duration.as_nanos() as u64); - let _ = self.io_size[READ_INDEX].datum.sample(len as u64); + let _ = self.io_size[READ_INDEX].datum.sample(len); self.reads.datum += 1; - self.bytes_read.datum += len as u64; + self.bytes_read.datum += len; return; } block::Result::Failure => FAILURE_INDEX, @@ -110,7 +110,7 @@ impl VirtualDiskStats { fn on_write_completion( &mut self, result: block::Result, - len: usize, + len: u64, duration: Duration, ) { let index = match result { @@ -118,9 +118,9 @@ impl VirtualDiskStats { let _ = self.io_latency[WRITE_INDEX] .datum .sample(duration.as_nanos() as u64); - let _ = self.io_size[WRITE_INDEX].datum.sample(len as u64); + let _ = self.io_size[WRITE_INDEX].datum.sample(len); self.writes.datum += 1; - self.bytes_written.datum += len as u64; + self.bytes_written.datum += len; return; } block::Result::Failure => FAILURE_INDEX, diff --git a/lib/propolis/src/block/crucible.rs b/lib/propolis/src/block/crucible.rs index 5ba04a14b..8f08fbdbb 100644 --- a/lib/propolis/src/block/crucible.rs +++ b/lib/propolis/src/block/crucible.rs @@ -82,7 +82,7 @@ impl WorkerState { readbuf: &mut Buffer, mem: &MemCtx, ) -> Result<(), Error> { - let block_size = self.info.block_size as usize; + let block_size = self.info.block_size as u64; match req.op { block::Operation::Read(off, len) => { @@ -94,7 +94,7 @@ impl WorkerState { // Perform one large read from crucible, and write from data into // mappings - readbuf.reset(len_blocks, block_size); + readbuf.reset(len_blocks as usize, block_size as usize); let _ = block.read(off_blocks, readbuf).await?; let mut nwritten = 0; @@ -104,8 +104,8 @@ impl WorkerState { )?; } - if nwritten != len { - return Err(Error::CopyError(nwritten, len)); + if nwritten as u64 != len { + return Err(Error::CopyError(nwritten as u64, len)); } } block::Operation::Write(off, len) => { @@ -120,7 +120,7 @@ impl WorkerState { // to crucible let maps = req.mappings(mem).ok_or_else(|| Error::BadGuestRegion)?; - let mut data = crucible::BytesMut::with_capacity(len); + let mut data = crucible::BytesMut::with_capacity(len as usize); let mut nread = 0; for mapping in maps { let n = mapping.read_bytes_uninit( @@ -133,8 +133,8 @@ impl WorkerState { } nread += n; } - if nread != len { - return Err(Error::CopyError(nread, len)); + if nread as u64 != len { + return Err(Error::CopyError(nread as u64, len)); } let _ = block.write(off_blocks, data).await?; @@ -406,7 +406,7 @@ pub enum Error { BlocksizeMismatch, #[error("copied length {0} did not match expectation {1}")] - CopyError(usize, usize), + CopyError(u64, u64), #[error("IO Error")] Io(#[from] io::Error), @@ -426,15 +426,15 @@ impl From for block::Result { /// Calculate offset (in crucible::Block form) and length in blocksize fn block_offset_count( - off_bytes: usize, - len_bytes: usize, - block_size: usize, -) -> Result<(crucible::BlockIndex, usize), Error> { + off_bytes: u64, + len_bytes: u64, + block_size: u64, +) -> Result<(crucible::BlockIndex, u64), Error> { if off_bytes.is_multiple_of(block_size) && len_bytes.is_multiple_of(block_size) { Ok(( - crucible::BlockIndex((off_bytes / block_size) as u64), + crucible::BlockIndex(off_bytes / block_size), len_bytes / block_size, )) } else { diff --git a/lib/propolis/src/block/file.rs b/lib/propolis/src/block/file.rs index 0d95aa43f..693afd7e0 100644 --- a/lib/propolis/src/block/file.rs +++ b/lib/propolis/src/block/file.rs @@ -94,7 +94,7 @@ impl SharedState { let nbytes = maps .preadv(self.fp.as_raw_fd(), off as i64) .map_err(|_| "io error")?; - if nbytes != len { + if nbytes as u64 != len { return Err("bad read length"); } } @@ -104,7 +104,7 @@ impl SharedState { let nbytes = maps .pwritev(self.fp.as_raw_fd(), off as i64) .map_err(|_| "io error")?; - if nbytes != len { + if nbytes as u64 != len { return Err("bad write length"); } } @@ -115,10 +115,9 @@ impl SharedState { } block::Operation::Discard(off, len) => { if let Some(mech) = self.discard_mech { - dkioc::do_discard(&self.fp, mech, off as u64, len as u64) - .map_err(|_| { - "io error while attempting to free block(s)" - })?; + dkioc::do_discard(&self.fp, mech, off, len).map_err( + |_| "io error while attempting to free block(s)", + )?; } else { unreachable!("handled above in processing_loop()"); } diff --git a/lib/propolis/src/block/in_memory.rs b/lib/propolis/src/block/in_memory.rs index 949412a45..fcd7bac0b 100644 --- a/lib/propolis/src/block/in_memory.rs +++ b/lib/propolis/src/block/in_memory.rs @@ -66,7 +66,7 @@ impl SharedState { })?; let bytes = self.bytes.lock().unwrap(); - process_read_request(&bytes, off as u64, len, &maps)?; + process_read_request(&bytes, off, len, &maps)?; } block::Operation::Write(off, len) => { if self.info.read_only { @@ -81,7 +81,7 @@ impl SharedState { })?; let mut bytes = self.bytes.lock().unwrap(); - process_write_request(&mut bytes, off as u64, len, &maps)?; + process_write_request(&mut bytes, off, len, &maps)?; } block::Operation::Flush => { // nothing to do @@ -182,11 +182,11 @@ impl block::Backend for InMemoryBackend { fn process_read_request( bytes: &[u8], offset: u64, - len: usize, + len: u64, mappings: &[SubMapping], ) -> Result<()> { let start = offset as usize; - let end = offset as usize + len; + let end = offset as usize + len as usize; if start >= bytes.len() || end > bytes.len() { return Err(std::io::Error::new( @@ -215,11 +215,11 @@ fn process_read_request( fn process_write_request( bytes: &mut [u8], offset: u64, - len: usize, + len: u64, mappings: &[SubMapping], ) -> Result<()> { let start = offset as usize; - let end = offset as usize + len; + let end = offset as usize + len as usize; if start >= bytes.len() || end > bytes.len() { return Err(std::io::Error::new( diff --git a/lib/propolis/src/block/mem_async.rs b/lib/propolis/src/block/mem_async.rs index d834bd388..9fd2cf9b7 100644 --- a/lib/propolis/src/block/mem_async.rs +++ b/lib/propolis/src/block/mem_async.rs @@ -66,7 +66,7 @@ impl SharedState { unsafe { let read_ptr = map.raw_writable()?; let len = map.len(); - seg.read(off + nread, read_ptr, len) + seg.read(off as usize + nread, read_ptr, len) .then_some(nread + len) } }) @@ -80,7 +80,7 @@ impl SharedState { unsafe { let write_ptr = map.raw_readable()?; let len = map.len(); - seg.write(off + nwritten, write_ptr, len) + seg.write(off as usize + nwritten, write_ptr, len) .then_some(nwritten + len) } }) diff --git a/lib/propolis/src/block/minder.rs b/lib/propolis/src/block/minder.rs index c9c62f576..72da29530 100644 --- a/lib/propolis/src/block/minder.rs +++ b/lib/propolis/src/block/minder.rs @@ -271,22 +271,16 @@ impl QueueMinder { let devqid = devq_id(self.device_id, self.queue_id); match req.op { Operation::Read(off, len) => { - probes::block_begin_read!(|| { - (devqid, id, off as u64, len as u64) - }); + probes::block_begin_read!(|| { (devqid, id, off, len) }); } Operation::Write(off, len) => { - probes::block_begin_write!(|| { - (devqid, id, off as u64, len as u64) - }); + probes::block_begin_write!(|| { (devqid, id, off, len) }); } Operation::Flush => { probes::block_begin_flush!(|| { (devqid, id) }); } Operation::Discard(off, len) => { - probes::block_begin_discard!(|| { - (devqid, id, off as u64, len as u64) - }); + probes::block_begin_discard!(|| { (devqid, id, off, len) }); } } let when_started = Instant::now(); diff --git a/lib/propolis/src/block/mod.rs b/lib/propolis/src/block/mod.rs index 674f2a985..08cce30d5 100644 --- a/lib/propolis/src/block/mod.rs +++ b/lib/propolis/src/block/mod.rs @@ -35,8 +35,8 @@ pub use attachment::{ }; pub use minder::{DeviceQueue, DeviceRequest}; -pub type ByteOffset = usize; -pub type ByteLen = usize; +pub type ByteOffset = u64; +pub type ByteLen = u64; /// When `block_size` is not specified in [BackendOpts], and the backend itself /// is not choosing a block size, a default of 512B is used. diff --git a/lib/propolis/src/hw/nvme/requests.rs b/lib/propolis/src/hw/nvme/requests.rs index 4821b47f0..a48fe6d1a 100644 --- a/lib/propolis/src/hw/nvme/requests.rs +++ b/lib/propolis/src/hw/nvme/requests.rs @@ -113,8 +113,7 @@ impl block::DeviceQueue for NvmeBlockQueue { )); let bufs = cmd.data(size, &mem).collect(); - let req = - Request::new_write(off as usize, size as usize, bufs); + let req = Request::new_write(off, size, bufs); return Some((req, permit, None)); } Ok(NvmCmd::Read(cmd)) => { @@ -138,8 +137,7 @@ impl block::DeviceQueue for NvmeBlockQueue { )); let bufs = cmd.data(size, &mem).collect(); - let req = - Request::new_read(off as usize, size as usize, bufs); + let req = Request::new_read(off, size, bufs); return Some((req, permit, None)); } Ok(NvmCmd::Flush) => { diff --git a/lib/propolis/src/hw/virtio/block.rs b/lib/propolis/src/hw/virtio/block.rs index 84c662019..f0ff5e6dd 100644 --- a/lib/propolis/src/hw/virtio/block.rs +++ b/lib/propolis/src/hw/virtio/block.rs @@ -24,10 +24,17 @@ use futures::future::BoxFuture; use lazy_static::lazy_static; /// Sizing for virtio-block is specified in 512B sectors -const SECTOR_SZ: usize = 512; - -/// Arbitrary limit to sectors permitted per discard request -const MAX_DISCARD_SECTORS: u32 = ((1024 * 1024) / SECTOR_SZ) as u32; +const SECTOR_SZ: u64 = 512; + +/// Arbitrary limit to sectors permitted per discard request. +/// +/// `u32` because that is the size of the register by which guests read this value. +const MAX_DISCARD_SECTORS: u32 = const { + let sectors: u64 = (1024 * 1024) / SECTOR_SZ; + // We can't `try_into` and unwrap in a const context, but we can assert and cast! + assert!(sectors <= u32::MAX as u64); + sectors as u32 +}; pub struct PciVirtioBlock { virtio_state: PciVirtioState, @@ -68,7 +75,7 @@ impl PciVirtioBlock { let total_bytes = info.total_size * u64::from(info.block_size); match id { BlockReg::Capacity => { - ro.write_u64(total_bytes / SECTOR_SZ as u64); + ro.write_u64(total_bytes / SECTOR_SZ); } BlockReg::SegMax => { // XXX: Copy the static limit from qemu for now @@ -82,7 +89,7 @@ impl PciVirtioBlock { // Arbitrarily limit to 1MiB (or the device size, if smaller) let sz = u32::min( MAX_DISCARD_SECTORS, - (info.total_size / SECTOR_SZ as u64) as u32, + (info.total_size / SECTOR_SZ) as u32, ); ro.write_u32(if info.supports_discard { sz } else { 0 }); } @@ -140,18 +147,18 @@ impl block::DeviceQueue for BlockVq { if !chain.read(&mut breq, &mem) { todo!("error handling"); } - let off = breq.sector as usize * SECTOR_SZ; + // TODO: handle mul overflow + let off = breq.sector * SECTOR_SZ; let req = match breq.rtype { VIRTIO_BLK_T_IN => { // should be (blocksize * 512) + 1 remaining writable byte for status // TODO: actually enforce block size - let blocks = (chain.remain_write_bytes() - 1) / SECTOR_SZ; + let blocks = + (chain.remain_write_bytes() - 1) as u64 / SECTOR_SZ; let sz = blocks * SECTOR_SZ; - if let Some(regions) = chain.writable_bufs(sz) { - probes::vioblk_read_enqueue!(|| ( - rid, off as u64, sz as u64 - )); + if let Some(regions) = chain.writable_bufs(sz as usize) { + probes::vioblk_read_enqueue!(|| (rid, off, sz)); Ok(( block::Request::new_read(off, sz, regions), CompletionToken { rid, chain }, @@ -163,13 +170,11 @@ impl block::DeviceQueue for BlockVq { } VIRTIO_BLK_T_OUT => { // should be (blocksize * 512) remaining read bytes - let blocks = chain.remain_read_bytes() / SECTOR_SZ; + let blocks = chain.remain_read_bytes() as u64 / SECTOR_SZ; let sz = blocks * SECTOR_SZ; - if let Some(regions) = chain.readable_bufs(sz) { - probes::vioblk_write_enqueue!(|| ( - rid, off as u64, sz as u64 - )); + if let Some(regions) = chain.readable_bufs(sz as usize) { + probes::vioblk_write_enqueue!(|| (rid, off, sz)); Ok(( block::Request::new_write(off, sz, regions), CompletionToken { rid, chain }, @@ -192,11 +197,9 @@ impl block::DeviceQueue for BlockVq { if !chain.read(&mut detail, &mem) { Err(chain) } else { - let off = detail.sector as usize * SECTOR_SZ; - let sz = detail.num_sectors as usize * SECTOR_SZ; - probes::vioblk_discard_enqueue!(|| ( - rid, off as u64, sz as u64, - )); + let off = detail.sector * SECTOR_SZ; + let sz = u64::from(detail.num_sectors) * SECTOR_SZ; + probes::vioblk_discard_enqueue!(|| (rid, off, sz)); Ok(( block::Request::new_discard(off, sz), CompletionToken { rid, chain },