Review

Author: will-j-wright

vm/devices/vmbus/vmbus_server/src/channels.rs

@@ -1672,6 +1672,8 @@ impl<'a, N: 'a + Notifier> ServerWithNotifier<'a, N> {
                 GpadlState::OfferedTearingDown => unreachable!(),
             }
         }
+
+        self.check_disconnected();
     }
 
     /// Initiates a state reset and a closing of all channels.

vm/devices/vmbus/vmbus_server/src/channels/saved_state.rs

@@ -184,6 +184,39 @@ impl<'a, N: 'a + Notifier> super::ServerWithNotifier<'a, N> {
         if let Some(saved) = saved.state {
             self.inner.state = saved.connection.restore()?;
 
+            // Restore server state, and resend server notifications if needed. If these notifications
+            // were processed before the save, it's harmless as the values will be the same.
+            let request = match self.inner.state {
+                super::ConnectionState::Connecting {
+                    info,
+                    next_action: _,
+                } => Some(super::ModifyConnectionRequest {
+                    version: Some(info.version.version as u32),
+                    interrupt_page: info.interrupt_page.into(),
+                    monitor_page: info.monitor_page.into(),
+                    target_message_vp: Some(info.target_message_vp),
+                    notify_relay: true,
+                }),
+                super::ConnectionState::Connected(info) => Some(super::ModifyConnectionRequest {
+                    version: None,
+                    monitor_page: info.monitor_page.into(),
+                    interrupt_page: info.interrupt_page.into(),
+                    target_message_vp: Some(info.target_message_vp),
+                    // If the save didn't happen while modifying, the relay doesn't need to be notified
+                    // of this info as it doesn't constitute a change, we're just restoring existing
+                    // connection state.
+                    notify_relay: info.modifying,
+                }),
+                // No action needed for these states; if disconnecting, check_disconnected will resend
+                // the reset request if needed.
+                super::ConnectionState::Disconnected
+                | super::ConnectionState::Disconnecting { .. } => None,
+            };
+
+            if let Some(request) = request {
+                self.notifier.modify_connection(request)?;
+            }
+
             for saved_channel in saved.channels {
                 self.inner.restore_one_channel(saved_channel)?;
             }
@@ -211,42 +244,6 @@ impl<'a, N: 'a + Notifier> super::ServerWithNotifier<'a, N> {
             self.inner.pending_messages.0.push_back(message.restore()?);
         }
 
-        // Restore server state, and resend server notifications if needed. If these notifications
-        // were processed before the save, it's harmless as the values will be the same.
-        let request = match self.inner.state {
-            super::ConnectionState::Connecting {
-                info,
-                next_action: _,
-            } => Some(super::ModifyConnectionRequest {
-                version: Some(info.version.version as u32),
-                interrupt_page: info.interrupt_page.into(),
-                monitor_page: info.monitor_page.into(),
-                target_message_vp: Some(info.target_message_vp),
-                notify_relay: true,
-            }),
-            super::ConnectionState::Connected(info) => Some(super::ModifyConnectionRequest {
-                version: None,
-                monitor_page: info.monitor_page.into(),
-                interrupt_page: info.interrupt_page.into(),
-                target_message_vp: Some(info.target_message_vp),
-                // If the save didn't happen while modifying, the relay doesn't need to be notified
-                // of this info as it doesn't constitute a change, we're just restoring existing
-                // connection state.
-                notify_relay: info.modifying,
-            }),
-            // No action needed for these states; if disconnecting, check_disconnected will resend
-            // the reset request if needed.
-            super::ConnectionState::Disconnected | super::ConnectionState::Disconnecting { .. } => {
-                None
-            }
-        };
-
-        if let Some(request) = request {
-            self.notifier.modify_connection(request)?;
-        }
-
-        self.check_disconnected();
-
         Ok(())
     }
 }

vm/devices/vmbus/vmbus_server/src/lib.rs

@@ -1711,7 +1711,6 @@ impl ServerTaskInner {
             Update::Set(value) => Some(value),
         };
 
-        // Force is used by restore because there may be restored channels in the open state.
         // TODO: can this check be moved into channels.rs?
         if self.channels.iter().any(|(_, c)| {
             matches!(