Remove Inventory mechanism

The Inventory abstraction in propolis-lib has outlived its necessity.
Created initially to keep devices in scope after VM initialization, it
was somewhat cumbersome to deal with, and was providing more friction
that value when it came to accessing the contained Entity bits.  With
its removal, the Instance abstraction can go as well, since it was
simply holding the Machine and the Inventory together.  In this new
world, propolis-lib consumers are left to hold Lifecycle (formerly
Entity) trait objects in whatever way seems appropriate to them.

With the Inventory gone, the parent-child relationships possible through
the former Entity trait are also on the chopping block.  The 440FX/PIIX4
chipset devices have been rearranged to more accurately reflect what is
present in "real" devices.

Author: pfmooney

bin/propolis-server/src/lib/initializer.rs

@@ -4,8 +4,7 @@
 
 use bitvec::prelude as bv;
 use futures::{SinkExt, StreamExt};
-use propolis::common::{GuestAddr, PAGE_SIZE};
-use propolis::inventory::Entity;
+use propolis::common::{GuestAddr, Lifecycle, PAGE_SIZE};
 use propolis::migrate::{
     MigrateCtx, MigrateStateError, Migrator, PayloadOffer, PayloadOffers,
 };
@@ -320,19 +319,18 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Send> DestinationProtocol<T> {
         info!(self.log(), "Devices: {devices:#?}");
 
         {
-            let instance_guard = self.vm_controller.instance().lock();
-            let inv = instance_guard.inventory();
-            let migrate_ctx = MigrateCtx {
-                mem: &instance_guard.machine().acc_mem.access().unwrap(),
-            };
+            let machine = self.vm_controller.machine();
+            let migrate_ctx =
+                MigrateCtx { mem: &machine.acc_mem.access().unwrap() };
             for device in devices {
                 info!(
                     self.log(),
                     "Applying state to device {}", device.instance_name
                 );
 
-                let target = inv
-                    .get_by_name(&device.instance_name)
+                let target = self
+                    .vm_controller
+                    .device_by_name(&device.instance_name)
                     .ok_or_else(|| {
                         MigrateError::UnknownDevice(
                             device.instance_name.clone(),
@@ -373,10 +371,7 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Send> DestinationProtocol<T> {
 
         // Take a snapshot of the host hrtime/wall clock time, then adjust
         // time data appropriately.
-        let vmm_hdl = {
-            let instance_guard = self.vm_controller.instance().lock();
-            &instance_guard.machine().hdl.clone()
-        };
+        let vmm_hdl = &self.vm_controller.machine().hdl.clone();
         let (dst_hrt, dst_wc) = vmm::time::host_time_snapshot(vmm_hdl)
             .map_err(|e| {
                 MigrateError::TimeData(format!(
@@ -451,7 +446,7 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Send> DestinationProtocol<T> {
 
     fn import_device(
         &self,
-        target: &Arc<dyn Entity>,
+        target: &Arc<dyn Lifecycle>,
         device: &Device,
         migrate_ctx: &MigrateCtx,
     ) -> Result<(), MigrateError> {
@@ -644,8 +639,8 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Send> DestinationProtocol<T> {
         addr: GuestAddr,
         buf: &[u8],
     ) -> Result<(), MigrateError> {
-        let instance_guard = self.vm_controller.instance().lock();
-        let memctx = instance_guard.machine().acc_mem.access().unwrap();
+        let machine = self.vm_controller.machine();
+        let memctx = machine.acc_mem.access().unwrap();
         let len = buf.len();
         memctx.write_from(addr, buf, len);
         Ok(())

bin/propolis-server/src/lib/migrate/destination.rs

@@ -4,7 +4,6 @@
 
 use futures::{SinkExt, StreamExt};
 use propolis::common::{GuestAddr, PAGE_SIZE};
-use propolis::inventory::Order;
 use propolis::migrate::{
     MigrateCtx, MigrateStateError, Migrator, PayloadOutputs,
 };
@@ -339,24 +338,23 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Send> SourceProtocol<T> {
         self.update_state(MigrationState::Device).await;
         let mut device_states = vec![];
         {
-            let instance_guard = self.vm_controller.instance().lock();
-            let migrate_ctx = MigrateCtx {
-                mem: &instance_guard.machine().acc_mem.access().unwrap(),
-            };
+            let machine = self.vm_controller.machine();
+            let migrate_ctx =
+                MigrateCtx { mem: &machine.acc_mem.access().unwrap() };
 
             // Collect together the serialized state for all the devices
-            instance_guard.inventory().for_each_node(Order::Pre, |_, rec| {
-                let entity = rec.entity();
+            self.vm_controller.for_each_device_fallible(|name, devop| {
                 let mut dev = Device {
-                    instance_name: rec.name().to_owned(),
+                    instance_name: name.to_string(),
                     payload: Vec::new(),
                 };
-                match entity.migrate() {
+                match devop.migrate() {
                     Migrator::NonMigratable => {
                         error!(self.log(),
                             "Can't migrate instance with non-migratable device ({})",
-                            rec.name());
-                        return Err(MigrateError::DeviceState(MigrateStateError::NonMigratable.to_string()));
+                            name);
+                        return Err(MigrateError::DeviceState(
+                                MigrateStateError::NonMigratable.to_string()));
                     },
                     // No device state needs to be trasmitted for 'Empty' devices
                     Migrator::Empty => {},
@@ -403,10 +401,7 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Send> SourceProtocol<T> {
 
     // Read and send over the time data
     async fn time_data(&mut self) -> Result<(), MigrateError> {
-        let vmm_hdl = {
-            let instance_guard = self.vm_controller.instance().lock();
-            &instance_guard.machine().hdl.clone()
-        };
+        let vmm_hdl = &self.vm_controller.machine().hdl.clone();
         let vm_time_data =
             vmm::time::export_time_data(vmm_hdl).map_err(|e| {
                 MigrateError::TimeData(format!(
@@ -547,8 +542,8 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Send> SourceProtocol<T> {
     async fn vmm_ram_bounds(
         &mut self,
     ) -> Result<RangeInclusive<GuestAddr>, MigrateError> {
-        let instance_guard = self.vm_controller.instance().lock();
-        let memctx = instance_guard.machine().acc_mem.access().unwrap();
+        let machine = self.vm_controller.machine();
+        let memctx = machine.acc_mem.access().unwrap();
         memctx.mem_bounds().ok_or(MigrateError::InvalidInstanceState)
     }
 
@@ -557,8 +552,7 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Send> SourceProtocol<T> {
         start_gpa: GuestAddr,
         bits: &mut [u8],
     ) -> Result<(), MigrateError> {
-        let instance_guard = self.vm_controller.instance().lock();
-        instance_guard
+        self.vm_controller
             .machine()
             .hdl
             .track_dirty_pages(start_gpa.0, bits)
@@ -570,8 +564,8 @@ impl<T: AsyncRead + AsyncWrite + Unpin + Send> SourceProtocol<T> {
         addr: GuestAddr,
         buf: &mut [u8],
     ) -> Result<(), MigrateError> {
-        let instance_guard = self.vm_controller.instance().lock();
-        let memctx = instance_guard.machine().acc_mem.access().unwrap();
+        let machine = self.vm_controller.machine();
+        let memctx = machine.acc_mem.access().unwrap();
         let len = buf.len();
         memctx.direct_read_into(addr, buf, len);
         Ok(())

bin/propolis-server/src/lib/migrate/source.rs

@@ -46,6 +46,10 @@ use crate::spec::{ServerSpecBuilder, ServerSpecBuilderError};
 use crate::vm::VmController;
 use crate::vnc::PropolisVncServer;
 
+pub(crate) type DeviceMap =
+    BTreeMap<String, Arc<dyn propolis::common::Lifecycle>>;
+pub(crate) type BlockBackendMap =
+    BTreeMap<String, Arc<dyn propolis::block::Backend>>;
 pub(crate) type CrucibleBackendMap =
     BTreeMap<uuid::Uuid, Arc<propolis::block::CrucibleBackend>>;
 
@@ -529,17 +533,14 @@ async fn instance_ensure_common(
         let vnc_fb = crate::vnc::RamFb::new(fb_spec);
 
         // Get a reference to the PS2 controller so that we can pass keyboard input.
-        let ps2ctrl = vm.ps2ctrl().unwrap();
+        let ps2ctrl = vm.ps2ctrl().clone();
 
         // Get a reference to the outward-facing VNC server in this process.
         let vnc_server = server_context.services.vnc_server.clone();
 
         // Initialize the Propolis VNC adapter with references to the VM's Instance,
         // framebuffer, and PS2 controller.
-        vnc_server
-            .server
-            .initialize(vnc_fb, Arc::clone(ps2ctrl), vm.clone())
-            .await;
+        vnc_server.server.initialize(vnc_fb, ps2ctrl, vm.clone()).await;
 
         // Hook up the framebuffer notifier to update the Propolis VNC adapter
         let notifier_server_ref = vnc_server.clone();

bin/propolis-server/src/lib/server.rs

@@ -47,13 +47,13 @@ pub(crate) trait VcpuTaskController {
 
 impl VcpuTasks {
     pub(crate) fn new(
-        instance: propolis::instance::InstanceGuard,
+        machine: &propolis::Machine,
         event_handler: Arc<super::vm::SharedVmState>,
         log: slog::Logger,
     ) -> Result<Self, VcpuTaskError> {
         let generation = Arc::new(AtomicUsize::new(0));
         let mut tasks = Vec::new();
-        for vcpu in instance.machine().vcpus.iter().map(Arc::clone) {
+        for vcpu in machine.vcpus.iter().map(Arc::clone) {
             let (task, ctrl) =
                 propolis::tasks::TaskHdl::new_held(Some(vcpu.barrier_fn()));
             let task_log = log.new(slog::o!("vcpu" => vcpu.id));