refactor: make net and block devices own metrics creation

Signed-off-by: aerosouund <aerosound161@gmail.com>

Author: aerosouund

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

@@ -24,7 +24,7 @@ use super::request::*;
 use super::{BLOCK_QUEUE_SIZES, SECTOR_SHIFT, SECTOR_SIZE, VirtioBlockError, io as block_io};
 use crate::devices::virtio::ActivateError;
 use crate::devices::virtio::block::CacheType;
-use crate::devices::virtio::block::virtio::metrics::{BlockDeviceMetrics, BlockMetricsPerDevice};
+use crate::devices::virtio::block::virtio::metrics::{BlockDeviceMetrics, METRICS};
 use crate::devices::virtio::device::{ActiveState, DeviceState, VirtioDevice, VirtioDeviceType};
 use crate::devices::virtio::generated::virtio_blk::{
     VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_BLK_ID_BYTES,
@@ -314,6 +314,11 @@ impl VirtioBlock {
         let config_space = ConfigSpace {
             capacity: disk_properties.nsectors.to_le(),
         };
+        let metrics = Arc::new(BlockDeviceMetrics::default());
+        METRICS
+            .write()
+            .unwrap()
+            .insert(config.drive_id.clone(), metrics.clone());
 
         Ok(VirtioBlock {
             avail_features,
@@ -334,7 +339,7 @@ impl VirtioBlock {
             disk: disk_properties,
             rate_limiter,
             is_io_engine_throttled: false,
-            metrics: BlockMetricsPerDevice::alloc(config.drive_id),
+            metrics: metrics,
         })
     }
 

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

@@ -86,50 +86,23 @@ use serde::{Serialize, Serializer};
 
 use crate::logger::{IncMetric, LatencyAggregateMetrics, SharedIncMetric};
 
-/// map of block drive id and metrics
-/// this should be protected by a lock before accessing.
-#[derive(Debug)]
-pub struct BlockMetricsPerDevice {
-    /// used to access per block device metrics
-    pub metrics: BTreeMap<String, Arc<BlockDeviceMetrics>>,
-}
-
-impl BlockMetricsPerDevice {
-    /// Allocate `BlockDeviceMetrics` for block device having
-    /// id `drive_id`. Also, allocate only if it doesn't
-    /// exist to avoid overwriting previously allocated data.
-    /// lock is always initialized so it is safe the unwrap
-    /// the lock without a check.
-    pub fn alloc(drive_id: String) -> Arc<BlockDeviceMetrics> {
-        Arc::clone(
-            METRICS
-                .write()
-                .unwrap()
-                .metrics
-                .entry(drive_id)
-                .or_insert_with(|| Arc::new(BlockDeviceMetrics::default())),
-        )
-    }
-}
-
 /// Pool of block-related metrics per device behind a lock to
 /// keep things thread safe. Since the lock is initialized here
 /// it is safe to unwrap it without any check.
-static METRICS: RwLock<BlockMetricsPerDevice> = RwLock::new(BlockMetricsPerDevice {
-    metrics: BTreeMap::new(),
-});
+pub static METRICS: RwLock<BTreeMap<String, Arc<BlockDeviceMetrics>>> =
+    RwLock::new(BTreeMap::new());
 
 /// This function facilitates aggregation and serialization of
 /// per block device metrics.
 pub fn flush_metrics<S: Serializer>(serializer: S) -> Result<S::Ok, S::Error> {
     let block_metrics = METRICS.read().unwrap();
-    let metrics_len = block_metrics.metrics.len();
+    let metrics_len = block_metrics.len();
     // +1 to accommodate aggregate block metrics
     let mut seq = serializer.serialize_map(Some(1 + metrics_len))?;
 
     let mut block_aggregated: BlockDeviceMetrics = BlockDeviceMetrics::default();
 
-    for (name, metrics) in block_metrics.metrics.iter() {
+    for (name, metrics) in block_metrics.iter() {
         let devn = format!("block_{}", name);
         // serialization will flush the metrics so aggregate before it.
         let m: &BlockDeviceMetrics = metrics;
@@ -247,10 +220,6 @@ pub mod tests {
         // devices on aarch64 but we stick to 19 to keep test common.
         const MAX_BLOCK_DEVICES: usize = 19;
 
-        // This is to make sure that RwLock for block::metrics::METRICS is good.
-        drop(METRICS.read().unwrap());
-        drop(METRICS.write().unwrap());
-
         // block::metrics::METRICS is in short RwLock on Vec of BlockDeviceMetrics.
         // Normally, pointer to unique entries of block::metrics::METRICS are stored
         // in Block device so that Block device can do self.metrics.* to
@@ -264,7 +233,7 @@ pub mod tests {
         let mut metrics: Vec<Arc<BlockDeviceMetrics>> = Vec::new();
         for i in 0..MAX_BLOCK_DEVICES {
             let devn: String = format!("drv{}", i);
-            metrics.push(BlockMetricsPerDevice::alloc(devn.clone()));
+            metrics.push(Arc::new(BlockDeviceMetrics::default()));
             // update IncMetric
             metrics[i].activate_fails.inc();
             // update SharedMetric
@@ -296,11 +265,7 @@ pub mod tests {
         // `test_max_block_dev_metrics` which also uses the same name.
         let devn = "drv0";
 
-        // This is to make sure that RwLock for block::metrics::METRICS is good.
-        drop(METRICS.read().unwrap());
-        drop(METRICS.write().unwrap());
-
-        let test_metrics = BlockMetricsPerDevice::alloc(String::from(devn));
+        let test_metrics = BlockDeviceMetrics::default();
         // Test to update IncMetrics
         test_metrics.activate_fails.inc();
         assert!(

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

@@ -11,14 +11,16 @@ use super::device::DiskProperties;
 use super::*;
 use crate::devices::virtio::block::persist::BlockConstructorArgs;
 use crate::devices::virtio::block::virtio::device::FileEngineType;
-use crate::devices::virtio::block::virtio::metrics::BlockMetricsPerDevice;
+use crate::devices::virtio::block::virtio::metrics::{BlockDeviceMetrics, METRICS};
 use crate::devices::virtio::device::{ActiveState, DeviceState, VirtioDeviceType};
 use crate::devices::virtio::generated::virtio_blk::VIRTIO_BLK_F_RO;
 use crate::devices::virtio::persist::VirtioDeviceState;
 use crate::rate_limiter::RateLimiter;
 use crate::rate_limiter::persist::RateLimiterState;
 use crate::snapshot::Persist;
 
+use std::sync::Arc;
+
 /// Holds info about block's file engine type. Gets saved in snapshot.
 #[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
 pub enum FileEngineTypeState {
@@ -114,6 +116,13 @@ impl Persist<'_> for VirtioBlock {
             capacity: disk_properties.nsectors.to_le(),
         };
 
+        let metrics = Arc::new(BlockDeviceMetrics::default());
+
+        METRICS
+            .write()
+            .unwrap()
+            .insert(state.id.clone(), metrics.clone());
+
         Ok(VirtioBlock {
             avail_features,
             acked_features,
@@ -133,7 +142,7 @@ impl Persist<'_> for VirtioBlock {
             disk: disk_properties,
             rate_limiter,
             is_io_engine_throttled: false,
-            metrics: BlockMetricsPerDevice::alloc(state.id.clone()),
+            metrics: metrics,
         })
     }
 }

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

@@ -29,7 +29,7 @@ use crate::devices::virtio::generated::virtio_ring::VIRTIO_RING_F_EVENT_IDX;
 use crate::devices::virtio::iovec::{
     IoVecBuffer, IoVecBufferMut, IoVecError, ParsedDescriptorChain,
 };
-use crate::devices::virtio::net::metrics::{NetDeviceMetrics, NetMetricsPerDevice};
+use crate::devices::virtio::net::metrics::{METRICS as NETMETRICS, NetDeviceMetrics};
 use crate::devices::virtio::net::tap::Tap;
 use crate::devices::virtio::net::{
     MAX_BUFFER_SIZE, NET_QUEUE_SIZES, NetError, NetQueue, RX_INDEX, TX_INDEX, generated,
@@ -303,6 +303,11 @@ impl Net {
             queue_evts.push(EventFd::new(libc::EFD_NONBLOCK).map_err(NetError::EventFd)?);
             queues.push(Queue::new(size));
         }
+        let metrics = Arc::new(NetDeviceMetrics::default());
+        NETMETRICS
+            .write()
+            .unwrap()
+            .insert(id.clone(), metrics.clone());
 
         Ok(Net {
             id: id.clone(),
@@ -320,7 +325,7 @@ impl Net {
             device_state: DeviceState::Inactive,
             activate_evt: EventFd::new(libc::EFD_NONBLOCK).map_err(NetError::EventFd)?,
             mmds_ns: None,
-            metrics: NetMetricsPerDevice::alloc(id),
+            metrics: metrics,
             tx_buffer: Default::default(),
             rx_buffer: RxBuffers::new()?,
         })

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

@@ -88,50 +88,22 @@ use serde::{Serialize, Serializer};
 
 use crate::logger::{IncMetric, LatencyAggregateMetrics, SharedIncMetric};
 
-/// map of network interface id and metrics
-/// this should be protected by a lock before accessing.
-#[derive(Debug)]
-pub struct NetMetricsPerDevice {
-    /// used to access per net device metrics
-    pub metrics: BTreeMap<String, Arc<NetDeviceMetrics>>,
-}
-
-impl NetMetricsPerDevice {
-    /// Allocate `NetDeviceMetrics` for net device having
-    /// id `iface_id`. Also, allocate only if it doesn't
-    /// exist to avoid overwriting previously allocated data.
-    /// lock is always initialized so it is safe the unwrap
-    /// the lock without a check.
-    pub fn alloc(iface_id: String) -> Arc<NetDeviceMetrics> {
-        Arc::clone(
-            METRICS
-                .write()
-                .unwrap()
-                .metrics
-                .entry(iface_id)
-                .or_insert_with(|| Arc::new(NetDeviceMetrics::default())),
-        )
-    }
-}
-
 /// Pool of Network-related metrics per device behind a lock to
 /// keep things thread safe. Since the lock is initialized here
 /// it is safe to unwrap it without any check.
-static METRICS: RwLock<NetMetricsPerDevice> = RwLock::new(NetMetricsPerDevice {
-    metrics: BTreeMap::new(),
-});
+pub static METRICS: RwLock<BTreeMap<String, Arc<NetDeviceMetrics>>> = RwLock::new(BTreeMap::new());
 
 /// This function facilitates aggregation and serialization of
 /// per net device metrics.
 pub fn flush_metrics<S: Serializer>(serializer: S) -> Result<S::Ok, S::Error> {
     let net_metrics = METRICS.read().unwrap();
-    let metrics_len = net_metrics.metrics.len();
+    let metrics_len = net_metrics.len();
     // +1 to accomodate aggregate net metrics
     let mut seq = serializer.serialize_map(Some(1 + metrics_len))?;
 
     let mut net_aggregated: NetDeviceMetrics = NetDeviceMetrics::default();
 
-    for (name, metrics) in net_metrics.metrics.iter() {
+    for (name, metrics) in net_metrics.iter() {
         let devn = format!("net_{}", name);
         // serialization will flush the metrics so aggregate before it.
         let m: &NetDeviceMetrics = metrics;
@@ -280,27 +252,29 @@ pub mod tests {
 
         for i in 0..MAX_NET_DEVICES {
             let devn: String = format!("eth{}", i);
-            NetMetricsPerDevice::alloc(devn.clone());
+
+            METRICS
+                .write()
+                .unwrap()
+                .insert(devn.to_string(), Arc::new(NetDeviceMetrics::default()));
+
             METRICS
                 .read()
                 .unwrap()
-                .metrics
                 .get(&devn)
                 .unwrap()
                 .activate_fails
                 .inc();
             METRICS
                 .read()
                 .unwrap()
-                .metrics
                 .get(&devn)
                 .unwrap()
                 .rx_bytes_count
                 .add(10);
             METRICS
                 .read()
                 .unwrap()
-                .metrics
                 .get(&devn)
                 .unwrap()
                 .tx_bytes_count
@@ -313,7 +287,6 @@ pub mod tests {
                 METRICS
                     .read()
                     .unwrap()
-                    .metrics
                     .get(&devn)
                     .unwrap()
                     .activate_fails
@@ -324,7 +297,6 @@ pub mod tests {
                 METRICS
                     .read()
                     .unwrap()
-                    .metrics
                     .get(&devn)
                     .unwrap()
                     .rx_bytes_count
@@ -335,7 +307,6 @@ pub mod tests {
                 METRICS
                     .read()
                     .unwrap()
-                    .metrics
                     .get(&devn)
                     .unwrap()
                     .tx_bytes_count
@@ -353,13 +324,14 @@ pub mod tests {
         drop(METRICS.read().unwrap());
         drop(METRICS.write().unwrap());
 
-        NetMetricsPerDevice::alloc(String::from(devn));
-        METRICS.read().unwrap().metrics.get(devn).unwrap();
+        METRICS
+            .write()
+            .unwrap()
+            .insert(devn.to_string(), Arc::new(NetDeviceMetrics::default()));
 
         METRICS
             .read()
             .unwrap()
-            .metrics
             .get(devn)
             .unwrap()
             .activate_fails
@@ -368,7 +340,6 @@ pub mod tests {
             METRICS
                 .read()
                 .unwrap()
-                .metrics
                 .get(devn)
                 .unwrap()
                 .activate_fails
@@ -378,7 +349,6 @@ pub mod tests {
             METRICS
                 .read()
                 .unwrap()
-                .metrics
                 .get(devn)
                 .unwrap()
                 .activate_fails
@@ -390,7 +360,6 @@ pub mod tests {
             METRICS
                 .read()
                 .unwrap()
-                .metrics
                 .get(devn)
                 .unwrap()
                 .activate_fails
@@ -400,7 +369,6 @@ pub mod tests {
             METRICS
                 .read()
                 .unwrap()
-                .metrics
                 .get(devn)
                 .unwrap()
                 .activate_fails
@@ -410,15 +378,13 @@ pub mod tests {
         METRICS
             .read()
             .unwrap()
-            .metrics
             .get(devn)
             .unwrap()
             .activate_fails
             .inc();
         METRICS
             .read()
             .unwrap()
-            .metrics
             .get(devn)
             .unwrap()
             .rx_bytes_count
@@ -427,7 +393,6 @@ pub mod tests {
             METRICS
                 .read()
                 .unwrap()
-                .metrics
                 .get(devn)
                 .unwrap()
                 .rx_bytes_count

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

@@ -496,6 +496,8 @@ mod tests {
             ],
         );
 
+        let entropy_event_fails = th.device().metrics.entropy_event_fails.count();
+        let entropy_event_count = th.device().metrics.entropy_event_count.count();
         let entropy_bytes = th.device().metrics.entropy_bytes.count();
         let host_rng_fails = th.device().metrics.host_rng_fails.count();
         assert_eq!(th.emulate_for_msec(100).unwrap(), 1);