Skip to content

Commit b96e570

Browse files
ShadowCurseShadowCurse
committed
refactor: remove SharedDeviceType
This wrapper was only used in the `update_from_restored_device` function which was instantly unwrapping the value and just adding device to the VmResources. Instead of this round trip just add devices directly since fields are public. Signed-off-by: Egor Lazarchuk <[email protected]>
1 parent 54d3759 commit b96e570

File tree

3 files changed

+22
-90
lines changed

3 files changed

+22
-90
lines changed

‎src/vmm/src/device_manager/pci_mngr.rs‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use event_manager::{MutEventSubscriber, SubscriberOps};
1010
use log::{debug, error, warn};
1111
use serde::{Deserialize, Serialize};
1212

13-
use super::persist::{MmdsState, SharedDeviceType};
13+
use super::persist::MmdsState;
1414
use crate::devices::pci::PciSegment;
1515
use crate::devices::virtio::balloon::Balloon;
1616
use crate::devices::virtio::balloon::persist::{BalloonConstructorArgs, BalloonState};
@@ -419,8 +419,8 @@ impl<'a> Persist<'a> for PciDevices {
419419

420420
constructor_args
421421
.vm_resources
422-
.update_from_restored_device(SharedDeviceType::Balloon(device.clone()))
423-
.unwrap();
422+
.balloon
423+
.set_device(device.clone());
424424

425425
pci_devices
426426
.restore_pci_device(
@@ -444,8 +444,8 @@ impl<'a> Persist<'a> for PciDevices {
444444

445445
constructor_args
446446
.vm_resources
447-
.update_from_restored_device(SharedDeviceType::VirtioBlock(device.clone()))
448-
.unwrap();
447+
.block
448+
.add_virtio_device(device.clone());
449449

450450
pci_devices
451451
.restore_pci_device(
@@ -494,8 +494,8 @@ impl<'a> Persist<'a> for PciDevices {
494494

495495
constructor_args
496496
.vm_resources
497-
.update_from_restored_device(SharedDeviceType::Network(device.clone()))
498-
.unwrap();
497+
.net_builder
498+
.add_device(device.clone());
499499

500500
pci_devices
501501
.restore_pci_device(
@@ -527,8 +527,8 @@ impl<'a> Persist<'a> for PciDevices {
527527

528528
constructor_args
529529
.vm_resources
530-
.update_from_restored_device(SharedDeviceType::Vsock(device.clone()))
531-
.unwrap();
530+
.vsock
531+
.set_device(device.clone());
532532

533533
pci_devices
534534
.restore_pci_device(
@@ -550,8 +550,8 @@ impl<'a> Persist<'a> for PciDevices {
550550

551551
constructor_args
552552
.vm_resources
553-
.update_from_restored_device(SharedDeviceType::Entropy(device.clone()))
554-
.unwrap();
553+
.entropy
554+
.set_device(device.clone());
555555

556556
pci_devices
557557
.restore_pci_device(

‎src/vmm/src/device_manager/persist.rs‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ use crate::devices::virtio::vsock::persist::{
4040
};
4141
use crate::devices::virtio::vsock::{Vsock, VsockError, VsockUnixBackend, VsockUnixBackendError};
4242
use crate::mmds::data_store::MmdsVersion;
43-
use crate::resources::{ResourcesError, VmResources};
43+
use crate::resources::VmResources;
4444
use crate::snapshot::Persist;
4545
use crate::vmm_config::mmds::MmdsConfigError;
4646
use crate::vstate::bus::BusError;
@@ -73,8 +73,6 @@ pub enum DevicePersistError {
7373
MmdsConfig(#[from] MmdsConfigError),
7474
/// Entropy: {0}
7575
Entropy(#[from] EntropyError),
76-
/// Resource misconfiguration: {0}. Is the snapshot file corrupted?
77-
ResourcesError(#[from] ResourcesError),
7876
/// Could not activate device: {0}
7977
DeviceActivation(#[from] ActivateError),
8078
}
@@ -128,17 +126,6 @@ pub struct DeviceStates {
128126
pub entropy_device: Option<VirtioDeviceState<EntropyState>>,
129127
}
130128

131-
/// A type used to extract the concrete `Arc<Mutex<T>>` for each of the device
132-
/// types when restoring from a snapshot.
133-
#[derive(Debug)]
134-
pub enum SharedDeviceType {
135-
VirtioBlock(Arc<Mutex<Block>>),
136-
Network(Arc<Mutex<Net>>),
137-
Balloon(Arc<Mutex<Balloon>>),
138-
Vsock(Arc<Mutex<Vsock<VsockUnixBackend>>>),
139-
Entropy(Arc<Mutex<Entropy>>),
140-
}
141-
142129
pub struct MMIODevManagerConstructorArgs<'a> {
143130
pub mem: &'a GuestMemoryMmap,
144131
pub vm: &'a Arc<Vm>,
@@ -422,7 +409,8 @@ impl<'a> Persist<'a> for MMIODeviceManager {
422409

423410
constructor_args
424411
.vm_resources
425-
.update_from_restored_device(SharedDeviceType::Balloon(device.clone()))?;
412+
.balloon
413+
.set_device(device.clone());
426414

427415
restore_helper(
428416
device.clone(),
@@ -444,7 +432,8 @@ impl<'a> Persist<'a> for MMIODeviceManager {
444432

445433
constructor_args
446434
.vm_resources
447-
.update_from_restored_device(SharedDeviceType::VirtioBlock(device.clone()))?;
435+
.block
436+
.add_virtio_device(device.clone());
448437

449438
restore_helper(
450439
device.clone(),
@@ -483,7 +472,8 @@ impl<'a> Persist<'a> for MMIODeviceManager {
483472

484473
constructor_args
485474
.vm_resources
486-
.update_from_restored_device(SharedDeviceType::Network(device.clone()))?;
475+
.net_builder
476+
.add_device(device.clone());
487477

488478
restore_helper(
489479
device.clone(),
@@ -512,7 +502,8 @@ impl<'a> Persist<'a> for MMIODeviceManager {
512502

513503
constructor_args
514504
.vm_resources
515-
.update_from_restored_device(SharedDeviceType::Vsock(device.clone()))?;
505+
.vsock
506+
.set_device(device.clone());
516507

517508
restore_helper(
518509
device.clone(),
@@ -536,7 +527,8 @@ impl<'a> Persist<'a> for MMIODeviceManager {
536527

537528
constructor_args
538529
.vm_resources
539-
.update_from_restored_device(SharedDeviceType::Entropy(device.clone()))?;
530+
.entropy
531+
.set_device(device.clone());
540532

541533
restore_helper(
542534
device.clone(),

‎src/vmm/src/resources.rs‎

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ use std::sync::{Arc, Mutex, MutexGuard};
88
use serde::{Deserialize, Serialize};
99

1010
use crate::cpu_config::templates::CustomCpuTemplate;
11-
use crate::device_manager::persist::SharedDeviceType;
1211
use crate::logger::info;
1312
use crate::mmds;
1413
use crate::mmds::data_store::{Mmds, MmdsVersion};
@@ -218,40 +217,6 @@ impl VmResources {
218217
Ok(mmds.lock().expect("Poisoned lock"))
219218
}
220219

221-
/// Updates the resources from a restored device (used for configuring resources when
222-
/// restoring from a snapshot).
223-
pub fn update_from_restored_device(
224-
&mut self,
225-
device: SharedDeviceType,
226-
) -> Result<(), ResourcesError> {
227-
match device {
228-
SharedDeviceType::VirtioBlock(block) => {
229-
self.block.add_virtio_device(block);
230-
}
231-
232-
SharedDeviceType::Network(network) => {
233-
self.net_builder.add_device(network);
234-
}
235-
236-
SharedDeviceType::Balloon(balloon) => {
237-
self.balloon.set_device(balloon);
238-
239-
if self.machine_config.huge_pages != HugePageConfig::None {
240-
return Err(ResourcesError::BalloonDevice(BalloonConfigError::HugePages));
241-
}
242-
}
243-
244-
SharedDeviceType::Vsock(vsock) => {
245-
self.vsock.set_device(vsock);
246-
}
247-
SharedDeviceType::Entropy(entropy) => {
248-
self.entropy.set_device(entropy);
249-
}
250-
}
251-
252-
Ok(())
253-
}
254-
255220
/// Add a custom CPU template to the VM resources
256221
/// to configure vCPUs.
257222
pub fn set_custom_cpu_template(&mut self, cpu_template: CustomCpuTemplate) {
@@ -1499,31 +1464,6 @@ mod tests {
14991464
.unwrap_err();
15001465
}
15011466

1502-
#[test]
1503-
fn test_negative_restore_balloon_device_with_huge_pages() {
1504-
let mut vm_resources = default_vm_resources();
1505-
vm_resources.balloon = BalloonBuilder::new();
1506-
vm_resources
1507-
.update_machine_config(&MachineConfigUpdate {
1508-
huge_pages: Some(HugePageConfig::Hugetlbfs2M),
1509-
..Default::default()
1510-
})
1511-
.unwrap();
1512-
let err = vm_resources
1513-
.update_from_restored_device(SharedDeviceType::Balloon(Arc::new(Mutex::new(
1514-
Balloon::new(128, false, 0, true).unwrap(),
1515-
))))
1516-
.unwrap_err();
1517-
assert!(
1518-
matches!(
1519-
err,
1520-
ResourcesError::BalloonDevice(BalloonConfigError::HugePages)
1521-
),
1522-
"{:?}",
1523-
err
1524-
);
1525-
}
1526-
15271467
#[test]
15281468
fn test_set_entropy_device() {
15291469
let mut vm_resources = default_vm_resources();

0 commit comments

Comments
 (0)