Skip to content

Commit

Permalink
server: initialize migration targets using their sources' specs
Browse files Browse the repository at this point in the history
Tweak the way VMs are initialized during live migration
  • Loading branch information
gjcolombo committed Dec 16, 2024
1 parent 220a6f3 commit 3af85e7
Show file tree
Hide file tree
Showing 9 changed files with 298 additions and 1,183 deletions.
938 changes: 0 additions & 938 deletions bin/propolis-server/src/lib/migrate/compat.rs

This file was deleted.

60 changes: 34 additions & 26 deletions bin/propolis-server/src/lib/migrate/destination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ use crate::migrate::probes;
use crate::migrate::{
Device, MigrateError, MigratePhase, MigrateRole, MigrationState, PageIter,
};
use crate::spec::Spec;
use crate::vm::ensure::{VmEnsureActive, VmEnsureNotStarted};
use crate::vm::state_publisher::{
ExternalStateUpdate, MigrationStateUpdate, StatePublisher,
Expand Down Expand Up @@ -171,21 +172,24 @@ impl<T: MigrateConn + Sync> DestinationProtocol for RonV0<T> {
let result = async {
// Run the sync phase to ensure that the source's instance spec is
// compatible with the spec supplied in the ensure parameters.
if let Err(e) = self.run_sync_phases(&mut ensure).await {
self.update_state(
ensure.state_publisher(),
MigrationState::Error,
);
let e = ensure.fail(e.into()).await;
return Err(e
.downcast::<MigrateError>()
.expect("original error was a MigrateError"));
}
let spec = match self.run_sync_phases(&mut ensure).await {
Ok(spec) => spec,
Err(e) => {
self.update_state(
ensure.state_publisher(),
MigrationState::Error,
);
let e = ensure.fail(e.into()).await;
return Err(e
.downcast::<MigrateError>()
.expect("original error was a MigrateError"));
}
};

// The sync phase succeeded, so it's OK to go ahead with creating
// the objects in the target's instance spec.
let mut objects_created =
ensure.create_objects().await.map_err(|e| {
ensure.create_objects_from_spec(spec).await.map_err(|e| {
MigrateError::TargetInstanceInitializationFailed(
e.to_string(),
)
Expand Down Expand Up @@ -260,14 +264,14 @@ impl<T: MigrateConn> RonV0<T> {
async fn run_sync_phases(
&mut self,
ensure_ctx: &mut VmEnsureNotStarted<'_>,
) -> Result<(), MigrateError> {
) -> Result<Spec, MigrateError> {
let step = MigratePhase::MigrateSync;

probes::migrate_phase_begin!(|| { step.to_string() });
self.sync(ensure_ctx).await?;
let result = self.sync(ensure_ctx).await;
probes::migrate_phase_end!(|| { step.to_string() });

Ok(())
result
}

async fn run_import_phases(
Expand Down Expand Up @@ -330,7 +334,7 @@ impl<T: MigrateConn> RonV0<T> {
async fn sync(
&mut self,
ensure_ctx: &mut VmEnsureNotStarted<'_>,
) -> Result<(), MigrateError> {
) -> Result<Spec, MigrateError> {
self.update_state(ensure_ctx.state_publisher(), MigrationState::Sync);
let preamble: Preamble = match self.read_msg().await? {
codec::Message::Serialized(s) => {
Expand All @@ -346,18 +350,22 @@ impl<T: MigrateConn> RonV0<T> {
}?;
info!(self.log(), "Destination read Preamble: {:?}", preamble);

if let Err(e) =
preamble.check_compatibility(&ensure_ctx.instance_spec().clone())
{
error!(
self.log(),
"source and destination instance specs incompatible";
"error" => #%e
);
return Err(MigrateError::InstanceSpecsIncompatible(e.to_string()));
}
let spec = match preamble.get_amended_spec(ensure_ctx.instance_spec()) {
Ok(spec) => spec,
Err(e) => {
error!(
self.log(),
"source and destination instance specs incompatible";
"error" => #%e
);
return Err(MigrateError::InstanceSpecsIncompatible(
e.to_string(),
));
}
};

self.send_msg(codec::Message::Okay).await
self.send_msg(codec::Message::Okay).await?;
Ok(spec)
}

async fn ram_push(
Expand Down
1 change: 0 additions & 1 deletion bin/propolis-server/src/lib/migrate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ use thiserror::Error;
use tokio::io::{AsyncRead, AsyncWrite};

mod codec;
mod compat;
pub mod destination;
mod memx;
mod preamble;
Expand Down
74 changes: 59 additions & 15 deletions bin/propolis-server/src/lib/migrate/preamble.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use propolis_api_types::instance_spec::VersionedInstanceSpec;
use propolis_api_types::instance_spec::{
v0::ComponentV0, VersionedInstanceSpec,
};
use serde::{Deserialize, Serialize};

use crate::spec::{self, api_spec_v0::ApiSpecError};
use crate::spec::{api_spec_v0::ApiSpecError, Spec, StorageBackend};

use super::MigrateError;

Expand All @@ -20,26 +22,68 @@ impl Preamble {
Preamble { instance_spec, blobs: Vec::new() }
}

/// Checks to see if the serialized spec in this preamble is compatible with
/// the supplied `other_spec`.
/// Given the instance spec in this preamble (transmitted from the migration
/// source), replace any Crucible and viona backends with their
///
/// This check runs on the destination.
pub fn check_compatibility(
pub fn get_amended_spec(
self,
other_spec: &spec::Spec,
) -> Result<(), MigrateError> {
let VersionedInstanceSpec::V0(v0_spec) = self.instance_spec;
let this_spec: spec::Spec =
v0_spec.try_into().map_err(|e: ApiSpecError| {
target_spec: &Spec,
) -> Result<Spec, MigrateError> {
let VersionedInstanceSpec::V0(mut source_spec) = self.instance_spec;
for disk in target_spec.disks.values() {
let StorageBackend::Crucible(crucible) = &disk.backend_spec else {
continue;
};

let Some(to_amend) =
source_spec.components.get_mut(disk.device_spec.backend_name())
else {
return Err(MigrateError::InstanceSpecsIncompatible(format!(
"replacement component {} not in source spec",
disk.device_spec.backend_name()
)));
};

if !matches!(to_amend, ComponentV0::CrucibleStorageBackend(_)) {
return Err(MigrateError::InstanceSpecsIncompatible(format!(
"component {} is not a Crucible backend in the source spec",
disk.device_spec.backend_name()
)));
}

*to_amend = ComponentV0::CrucibleStorageBackend(crucible.clone());
}

for nic in target_spec.nics.values() {
let Some(to_amend) =
source_spec.components.get_mut(&nic.device_spec.backend_name)
else {
return Err(MigrateError::InstanceSpecsIncompatible(format!(
"replacement component {} not in source spec",
nic.device_spec.backend_name
)));
};

if !matches!(to_amend, ComponentV0::VirtioNetworkBackend(_)) {
return Err(MigrateError::InstanceSpecsIncompatible(format!(
"component {} is not a virtio network backend \
in the source spec",
nic.device_spec.backend_name
)));
}

*to_amend =
ComponentV0::VirtioNetworkBackend(nic.backend_spec.clone());
}

let amended_spec =
source_spec.try_into().map_err(|e: ApiSpecError| {
MigrateError::PreambleParse(e.to_string())
})?;

this_spec.is_migration_compatible(other_spec).map_err(|e| {
MigrateError::InstanceSpecsIncompatible(e.to_string())
})?;

// TODO: Compare opaque blobs.

Ok(())
Ok(amended_spec)
}
}
11 changes: 0 additions & 11 deletions bin/propolis-server/src/lib/spec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,17 +294,6 @@ pub struct SoftNpu {
pub p9fs: Option<P9fs>,
}

#[cfg(feature = "falcon")]
impl SoftNpu {
/// Returns `true` if this struct specifies at least one SoftNPU component.
pub fn has_components(&self) -> bool {
self.pci_port.is_some()
|| self.p9_device.is_some()
|| self.p9fs.is_some()
|| !self.ports.is_empty()
}
}

struct ParsedDiskRequest {
name: String,
disk: Disk,
Expand Down
Loading

0 comments on commit 3af85e7

Please sign in to comment.