Remove MigrateStart instance state request (#332)

This request sets a flag in the VM controller that lets a VM act as a migration source if its state otherwise permits it. There's no way to clear the flag, which makes it tough to write an undo action for this in the Nexus live migration saga that (presumably) would issue this call.

Since the request has no other side effects, the VM controller already rejects other invalid requests to migrate out, and sled agent doesn't presently use this request type, just remove it entirely.

Tested with an ad hoc migration & PHD tests.

Author: gjcolombo

bin/propolis-cli/src/main.rs

@@ -441,14 +441,6 @@ async fn migrate_instance(
         cloud_init_bytes: None,
     };
 
-    // Get the source instance ready
-    src_client
-        .instance_state_put(InstanceStateRequested::MigrateStart)
-        .await
-        .with_context(|| {
-        anyhow!("failed to place src instance in migrate start state")
-    })?;
-
     // Initiate the migration via the destination instance
     let migration_id = dst_client
         .instance_ensure(&request)

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

@@ -133,9 +133,6 @@ impl InstanceContext {
                     self.state = api::InstanceState::Stopped;
                     Ok(())
                 }
-                api::InstanceStateRequested::MigrateStart => {
-                    unimplemented!("migration not yet implemented")
-                }
             },
         }
     }

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

@@ -69,9 +69,6 @@ pub enum VmControllerError {
     #[error("The instance has a pending request to halt")]
     InstanceHaltPending,
 
-    #[error("Instance is not marked as a possible migration source")]
-    NotMarkedMigrationSource,
-
     #[error("Instance is already a migration source")]
     AlreadyMigrationSource,
 
@@ -114,7 +111,6 @@ impl From<VmControllerError> for dropshot::HttpError {
         use dropshot::HttpError;
         match vm_error {
             VmControllerError::AlreadyMigrationSource
-            | VmControllerError::NotMarkedMigrationSource
             | VmControllerError::InvalidRequestForMigrationSource(_)
             | VmControllerError::MigrationTargetInProgress
             | VmControllerError::MigrationTargetFailed
@@ -322,12 +318,6 @@ struct SharedVmStateInner {
     /// that's already running is forbidden.
     lifecycle_stage: LifecycleStage,
 
-    /// True if the instance previously received an API call denoting that it
-    /// should expect a request to act as a live migration source. If false,
-    /// incoming requests to migrate from a prospective destination should be
-    /// rejected.
-    marked_migration_source: bool,
-
     /// True if the instance's request queue has a pending request to migrate to
     /// another instance. If true, external API requests that may need to be
     /// serviced by the target should be blocked until this is cleared. (Clients
@@ -357,7 +347,6 @@ impl SharedVmStateInner {
     fn new() -> Self {
         Self {
             lifecycle_stage: LifecycleStage::NotStarted(StartupStage::ColdBoot),
-            marked_migration_source: false,
             migrate_from_pending: false,
             halt_pending: false,
             external_request_queue: VecDeque::new(),
@@ -678,13 +667,10 @@ impl VmController {
         let mut inner = self.worker_state.inner.lock().unwrap();
         match inner.lifecycle_stage {
             LifecycleStage::NotStarted(_) => {
-                assert!(!inner.marked_migration_source);
                 Err(VmControllerError::InstanceNotActive)
             }
             LifecycleStage::Active => {
-                if !inner.marked_migration_source {
-                    Err(VmControllerError::NotMarkedMigrationSource)
-                } else if inner.migrate_from_pending {
+                if inner.migrate_from_pending {
                     Err(VmControllerError::AlreadyMigrationSource)
                 } else if inner.halt_pending {
                     Err(VmControllerError::InstanceHaltPending)
@@ -957,26 +943,6 @@ impl VmController {
                     }
                 }
             }
-
-            // Requests to mark the VM as a migration source require the VM to
-            // be active.
-            ApiInstanceStateRequested::MigrateStart => {
-                match inner.lifecycle_stage {
-                    LifecycleStage::NotStarted(_) => {
-                        Err(VmControllerError::InvalidStageForRequest(
-                            requested,
-                            inner.lifecycle_stage,
-                        ))
-                    }
-                    LifecycleStage::Active => {
-                        inner.marked_migration_source = true;
-                        Ok(())
-                    }
-                    LifecycleStage::NoLongerActive => {
-                        Err(VmControllerError::InstanceNotActive)
-                    }
-                }
-            }
         }
     }
 

lib/propolis-client/src/handmade/api.rs

@@ -138,7 +138,6 @@ pub enum InstanceStateRequested {
     Run,
     Stop,
     Reboot,
-    MigrateStart,
 }
 
 /// Current state of an Instance.

openapi/propolis-server.json

@@ -986,8 +986,7 @@
         "enum": [
           "Run",
           "Stop",
-          "Reboot",
-          "MigrateStart"
+          "Reboot"
         ]
       },
       "MigrationState": {

phd-tests/framework/src/test_vm/mod.rs

@@ -242,14 +242,6 @@ impl TestVm {
         })
     }
 
-    /// Moves this VM into the migrate-start state.
-    fn start_migrate(&self) -> StdResult<(), PropolisClientError> {
-        self.rt.block_on(async {
-            self.put_instance_state_async(InstanceStateRequested::MigrateStart)
-                .await
-        })
-    }
-
     async fn put_instance_state_async(
         &self,
         state: InstanceStateRequested,
@@ -303,7 +295,6 @@ impl TestVm {
                     source.server.server_addr()
                 );
 
-                source.start_migrate()?;
                 let console = self.rt.block_on(async {
                     self.instance_ensure_async(Some(
                         InstanceMigrateInitiateRequest {