Remove Inventory mechanism #639
Conversation
|
Hmm, it looks like the "migration from base" PHD tests have actually caught an incompatibility! |
Indeed they have! That failure is expected, given the device shuffling. |
Ah, okay. It's good to know the tests are working, in any case. :) |
| spec: &'a InstanceSpecV0, | ||
| producer_registry: Option<ProducerRegistry>, | ||
| pub(crate) log: slog::Logger, | ||
| pub(crate) machine: &'a Machine, |
There was a problem hiding this comment.
Not necessary for this PR, just a forward-looking question: how challenging do you think it'd be to have a fake Machine? I'm thinking about what it'd take to write unit tests of the form "I added this initializer function and expect that when I call it with spec X the appropriate things will show up in maps A, B, and C."
There was a problem hiding this comment.
I think that as the core abstractions become more polished, such a thing will become easier. Machine is obviously doing much less (as of this change) than Instance -- mostly just holding references to top-level items. It seems likely that with a bit more clean-up (particularly around memory maps, and mediating access to the VmmHdl) that Machine could be discarded as well.
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.
After the integration of #639, the CI tests report lingering VMM resources after conclusion of the run.
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.