From a45e458c50c202ccc2f96b50cea6fde1054f2b41 Mon Sep 17 00:00:00 2001 From: lizhaoxin1 Date: Sat, 29 Jan 2022 10:53:58 +0800 Subject: [PATCH] vm-migration: Add start_migration() to Migratable trait In order to clearly decouple when the migration is started compared to when the dirty logging is started, we introduce a new method to the Migratable trait. This clarifies the semantics as we don't end up using start_dirty_log() for identifying when the migration has been started. And similarly, we rely on the already existing complete_migration() method to know when the migration has been ended. A bug was reported when running a local migration with a vhost-user-net device in server mode. The reason was because the migration_started variable was never set to "true", since the start_dirty_log() function was never invoked. Signed-off-by: lizhaoxin1 Signed-off-by: Sebastien Boeuf --- virtio-devices/src/vhost_user/blk.rs | 4 ++++ virtio-devices/src/vhost_user/fs.rs | 4 ++++ virtio-devices/src/vhost_user/mod.rs | 9 +++++++-- virtio-devices/src/vhost_user/net.rs | 4 ++++ vm-migration/src/lib.rs | 4 ++++ vmm/src/device_manager.rs | 9 +++++++++ vmm/src/lib.rs | 3 +++ vmm/src/vm.rs | 5 +++++ 8 files changed, 40 insertions(+), 2 deletions(-) diff --git a/virtio-devices/src/vhost_user/blk.rs b/virtio-devices/src/vhost_user/blk.rs index a6f0533ab..d7ed7a534 100644 --- a/virtio-devices/src/vhost_user/blk.rs +++ b/virtio-devices/src/vhost_user/blk.rs @@ -417,6 +417,10 @@ impl Migratable for Blk { self.vu_common.dirty_log(&self.guest_memory) } + fn start_migration(&mut self) -> std::result::Result<(), MigratableError> { + self.vu_common.start_migration() + } + fn complete_migration(&mut self) -> std::result::Result<(), MigratableError> { self.vu_common .complete_migration(self.common.kill_evt.take()) diff --git a/virtio-devices/src/vhost_user/fs.rs b/virtio-devices/src/vhost_user/fs.rs index 83d08b105..7fc208677 100644 --- a/virtio-devices/src/vhost_user/fs.rs +++ b/virtio-devices/src/vhost_user/fs.rs @@ -698,6 +698,10 @@ impl Migratable for Fs { self.vu_common.dirty_log(&self.guest_memory) } + fn start_migration(&mut self) -> std::result::Result<(), MigratableError> { + self.vu_common.start_migration() + } + fn complete_migration(&mut self) -> std::result::Result<(), MigratableError> { self.vu_common .complete_migration(self.common.kill_evt.take()) diff --git a/virtio-devices/src/vhost_user/mod.rs b/virtio-devices/src/vhost_user/mod.rs index 1e2509fab..bf757a6ca 100644 --- a/virtio-devices/src/vhost_user/mod.rs +++ b/virtio-devices/src/vhost_user/mod.rs @@ -451,7 +451,6 @@ impl VhostUserCommon { &mut self, guest_memory: &Option>, ) -> std::result::Result<(), MigratableError> { - self.migration_started = true; if let Some(vu) = &self.vu { if let Some(guest_memory) = guest_memory { let last_ram_addr = guest_memory.memory().last_addr().raw_value(); @@ -475,7 +474,6 @@ impl VhostUserCommon { } pub fn stop_dirty_log(&mut self) -> std::result::Result<(), MigratableError> { - self.migration_started = false; if let Some(vu) = &self.vu { vu.lock().unwrap().stop_dirty_log().map_err(|e| { MigratableError::StopDirtyLog(anyhow!( @@ -509,10 +507,17 @@ impl VhostUserCommon { } } + pub fn start_migration(&mut self) -> std::result::Result<(), MigratableError> { + self.migration_started = true; + Ok(()) + } + pub fn complete_migration( &mut self, kill_evt: Option, ) -> std::result::Result<(), MigratableError> { + self.migration_started = false; + // Make sure the device thread is killed in order to prevent from // reconnections to the socket. if let Some(kill_evt) = kill_evt { diff --git a/virtio-devices/src/vhost_user/net.rs b/virtio-devices/src/vhost_user/net.rs index 08a8d92d7..40b77b934 100644 --- a/virtio-devices/src/vhost_user/net.rs +++ b/virtio-devices/src/vhost_user/net.rs @@ -488,6 +488,10 @@ impl Migratable for Net { self.vu_common.dirty_log(&self.guest_memory) } + fn start_migration(&mut self) -> std::result::Result<(), MigratableError> { + self.vu_common.start_migration() + } + fn complete_migration(&mut self) -> std::result::Result<(), MigratableError> { self.vu_common .complete_migration(self.common.kill_evt.take()) diff --git a/vm-migration/src/lib.rs b/vm-migration/src/lib.rs index a0a39260f..e0b38c867 100644 --- a/vm-migration/src/lib.rs +++ b/vm-migration/src/lib.rs @@ -305,6 +305,10 @@ pub trait Migratable: Send + Pausable + Snapshottable + Transportable { Ok(MemoryRangeTable::default()) } + fn start_migration(&mut self) -> std::result::Result<(), MigratableError> { + Ok(()) + } + fn complete_migration(&mut self) -> std::result::Result<(), MigratableError> { Ok(()) } diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index e1e6a906f..a52e324a5 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -4221,6 +4221,15 @@ impl Migratable for DeviceManager { Ok(MemoryRangeTable::new_from_tables(tables)) } + fn start_migration(&mut self) -> std::result::Result<(), MigratableError> { + for (_, device_node) in self.device_tree.lock().unwrap().iter() { + if let Some(migratable) = &device_node.migratable { + migratable.lock().unwrap().start_migration()?; + } + } + Ok(()) + } + fn complete_migration(&mut self) -> std::result::Result<(), MigratableError> { for (_, device_node) in self.device_tree.lock().unwrap().iter() { if let Some(migratable) = &device_node.migratable { diff --git a/vmm/src/lib.rs b/vmm/src/lib.rs index 600d7e720..0de2249cc 100644 --- a/vmm/src/lib.rs +++ b/vmm/src/lib.rs @@ -1100,6 +1100,9 @@ impl Vmm { ))); } + // Let every Migratable object know about the migration being started. + vm.start_migration()?; + if send_data_migration.local { // Now pause VM vm.pause()?; diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index 191d63b8a..933205e4f 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -2675,6 +2675,11 @@ impl Migratable for Vm { ])) } + fn start_migration(&mut self) -> std::result::Result<(), MigratableError> { + self.memory_manager.lock().unwrap().start_migration()?; + self.device_manager.lock().unwrap().start_migration() + } + fn complete_migration(&mut self) -> std::result::Result<(), MigratableError> { self.memory_manager.lock().unwrap().complete_migration()?; self.device_manager.lock().unwrap().complete_migration()