Skip to content

Commit

Permalink
lib/server: allow migration of in-memory block backends (#826)
Browse files Browse the repository at this point in the history
Register in-memory block backends for lifecycle callouts and teach them to
export and import their contents. Add a new PHD test for this behavior.

Tighten some bolts in PHD to support the new test:

- Add a helper to run Powershell cmdlets in Windows VMs.
- Change the Ubuntu 22.04 adapter's login sequence so that tests run as
  root.
  • Loading branch information
gjcolombo authored Dec 18, 2024
1 parent d4529fd commit 7d94141
Show file tree
Hide file tree
Showing 10 changed files with 342 additions and 48 deletions.
6 changes: 5 additions & 1 deletion bin/propolis-server/src/lib/initializer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ impl<'a> MachineInitializer<'a> {
}

async fn create_storage_backend_from_spec(
&self,
&mut self,
backend_spec: &StorageBackend,
backend_id: &SpecKey,
nexus_client: &Option<NexusClient>,
Expand Down Expand Up @@ -573,6 +573,10 @@ impl<'a> MachineInitializer<'a> {
)
.context("failed to create in-memory storage backend")?;

// In-memory backends need to be registered for lifecycle
// notifications so that they can export/import changes to the
// backing disk across migrations.
self.devices.insert(backend_id.clone(), be.clone());
Ok(StorageBackendInstance { be, crucible: None })
}
}
Expand Down
2 changes: 1 addition & 1 deletion bin/propolis-server/src/lib/stats/virtual_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ impl KstatTarget for VirtualMachine {
.find_map(|(_, kstat, data)| {
kstat_instance_from_instance_id(kstat, data, &self.vm_name)
})
.ok_or_else(|| Error::NoSuchKstat)?;
.ok_or(Error::NoSuchKstat)?;

// Armed with the kstat instance, find all relevant metrics related to
// this particular VM. For now, we produce only vCPU usage metrics, but
Expand Down
70 changes: 70 additions & 0 deletions lib/propolis/src/block/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ use std::sync::{Arc, Mutex};

use crate::accessors::MemAccessor;
use crate::block;
use crate::common::Lifecycle;
use crate::migrate::{
MigrateCtx, MigrateSingle, MigrateStateError, Migrator, PayloadOffer,
PayloadOutput,
};
use crate::tasks::ThreadGroup;
use crate::vmm::{MemCtx, SubMapping};

Expand Down Expand Up @@ -232,3 +237,68 @@ fn process_write_request(

Ok(())
}

impl Lifecycle for InMemoryBackend {
fn type_name(&self) -> &'static str {
"in-memory-storage"
}

fn migrate(&self) -> Migrator {
Migrator::Single(self)
}
}

impl MigrateSingle for InMemoryBackend {
fn export(
&self,
_ctx: &MigrateCtx,
) -> std::result::Result<PayloadOutput, MigrateStateError> {
let bytes = self.state.bytes.lock().unwrap();
Ok(migrate::InMemoryBlockBackendV1 { bytes: bytes.clone() }.into())
}

fn import(
&self,
mut offer: PayloadOffer,
_ctx: &MigrateCtx,
) -> std::result::Result<(), MigrateStateError> {
let data: migrate::InMemoryBlockBackendV1 = offer.parse()?;
let mut guard = self.state.bytes.lock().unwrap();
if guard.len() != data.bytes.len() {
return Err(MigrateStateError::ImportFailed(format!(
"imported in-memory block backend data has length {}, \
but backend's original length was {}",
data.bytes.len(),
guard.len()
)));
}

*guard = data.bytes;
Ok(())
}
}

mod migrate {
use serde::{Deserialize, Serialize};

use crate::migrate::{Schema, SchemaId};

#[derive(Serialize, Deserialize)]
pub struct InMemoryBlockBackendV1 {
pub(super) bytes: Vec<u8>,
}

impl std::fmt::Debug for InMemoryBlockBackendV1 {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("InMemoryBlockBackendV1")
.field("bytes", &"<redacted>".to_string())
.finish()
}
}

impl Schema<'_> for InMemoryBlockBackendV1 {
fn id() -> SchemaId {
("in-memory-block-backend", 1)
}
}
}
5 changes: 4 additions & 1 deletion phd-tests/framework/src/artifacts/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,10 @@ fn file_hash_equals(
path: impl AsRef<std::path::Path>,
expected_digest: &str,
) -> anyhow::Result<()> {
let file = File::open(path)?;
let file = File::open(&path).with_context(|| {
format!("checking hash for file {}", path.as_ref().display())
})?;

let mut reader = BufReader::new(file);
hash_equals(&mut reader, expected_digest)
}
Expand Down
22 changes: 17 additions & 5 deletions phd-tests/framework/src/disk/fat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,13 +143,26 @@ impl FatFilesystem {
Self { files: vec![], sectors_remaining: total_usable_sectors() }
}

/// Converts the supplied `contents` string slice to bytes and adds it to
/// the filesystem using [`Self::add_file_from_bytes`].
///
/// The supplied `filename` must not contain any path separators (the `/`
/// character).
pub fn add_file_from_str(
&mut self,
filename: &str,
contents: &str,
) -> Result<(), Error> {
self.add_file_from_bytes(filename, contents.as_bytes())
}

/// Adds a file with the supplied `contents` that will appear in the root
/// directory of the generated file system. The given `filename` must not
/// contain any path separators (the `/` character).
pub fn add_file_from_str(
pub fn add_file_from_bytes(
&mut self,
filename: &str,
contents: &str,
contents: &[u8],
) -> Result<(), Error> {
// The `fatfs` crate will break paths containing separators into their
// component directories before trying to create the requested file in
Expand All @@ -163,8 +176,7 @@ impl FatFilesystem {
return Err(Error::PathSeparatorInFilename(filename.to_owned()));
}

let bytes = contents.as_bytes();
let sectors_needed = Sectors::needed_for_bytes(bytes.len());
let sectors_needed = Sectors::needed_for_bytes(contents.len());
if sectors_needed > self.sectors_remaining {
Err(Error::NoSpace {
required: sectors_needed.0,
Expand All @@ -173,7 +185,7 @@ impl FatFilesystem {
} else {
self.files.push(File {
name: filename.to_owned(),
contents: bytes.to_vec(),
contents: contents.to_vec(),
});

self.sectors_remaining -= sectors_needed;
Expand Down
2 changes: 1 addition & 1 deletion phd-tests/framework/src/guest_os/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ mod debian11_nocloud;
mod linux;
mod shell_commands;
mod ubuntu22_04;
mod windows;
pub mod windows;
mod windows_server_2016;
mod windows_server_2019;
mod windows_server_2022;
Expand Down
6 changes: 5 additions & 1 deletion phd-tests/framework/src/guest_os/ubuntu22_04.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ impl GuestOs for Ubuntu2204 {
CommandSequenceEntry::write_str("ubuntu"),
CommandSequenceEntry::wait_for("Password: "),
CommandSequenceEntry::write_str("1!Passw0rd"),
CommandSequenceEntry::wait_for("ubuntu@ubuntu:~$"),
CommandSequenceEntry::write_str("sudo bash\n"),
CommandSequenceEntry::wait_for("root@ubuntu:/home/ubuntu#"),
CommandSequenceEntry::write_str("cd ~\n"),
CommandSequenceEntry::wait_for(self.get_shell_prompt()),
])
.extend(super::linux::stty_enable_long_lines(self))
}

fn get_shell_prompt(&self) -> &'static str {
"ubuntu@ubuntu:~$"
"root@ubuntu:~#"
}

fn read_only_fs(&self) -> bool {
Expand Down
48 changes: 47 additions & 1 deletion phd-tests/framework/src/guest_os/windows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,56 @@
// 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/.

//! Helper functions for generating Windows guest OS adaptations.
//! Functionality common to all Windows guests.
use crate::TestVm;

use super::{CommandSequence, CommandSequenceEntry, GuestOsKind};

use tracing::info;

/// A wrapper that provides Windows-specific extensions to the core `TestVm`
/// implementation.
pub struct WindowsVm<'a> {
/// The VM being extended by this structure. The framework is required to
/// ensure that the VM is actually configured to run a Windows guest OS.
pub(crate) vm: &'a TestVm,
}

impl WindowsVm<'_> {
/// Runs `cmd` as a Powershell command.
pub async fn run_powershell_command(
&self,
cmd: &str,
) -> anyhow::Result<String> {
assert!(self.vm.guest_os_kind().is_windows());

info!(cmd, "executing Powershell command");

// Use Powershell's -encodedCommand switch to keep important Powershell
// sigils in the command (like "$") from being interpreted by whatever
// shell is being used to invoke Powershell. This switch expects that
// the encoded string will decode into a UTF-16 string; `str`s are, of
// course, UTF-8, so switch encodings before converting to base64.
let utf16 = cmd.encode_utf16().collect::<Vec<u16>>();
let base64 = base64::Engine::encode(
&base64::engine::general_purpose::STANDARD,
unsafe { utf16.align_to::<u8>().1 },
);

let cmd = format!("powershell -encodedCommand {base64}");
self.vm.run_shell_command(&cmd).await
}
}

impl std::ops::Deref for WindowsVm<'_> {
type Target = TestVm;

fn deref(&self) -> &Self::Target {
self.vm
}
}

const CYGWIN_CMD: &str = "C:\\cygwin\\cygwin.bat\r";

/// Prepends a `reset` command to the shell command supplied in `cmd`. Windows
Expand Down
9 changes: 8 additions & 1 deletion phd-tests/framework/src/test_vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ use std::{

use crate::{
guest_os::{
self, CommandSequence, CommandSequenceEntry, GuestOs, GuestOsKind,
self, windows::WindowsVm, CommandSequence, CommandSequenceEntry,
GuestOs, GuestOsKind,
},
serial::{BufferKind, SerialConsole},
test_vm::{
Expand Down Expand Up @@ -387,6 +388,12 @@ impl TestVm {
self.spec.guest_os_kind
}

/// If this VM is running a Windows guest, returns a wrapper that provides
/// Windows-specific VM functions.
pub fn get_windows_vm(&self) -> Option<WindowsVm> {
self.guest_os_kind().is_windows().then_some(WindowsVm { vm: self })
}

/// Sets the VM to the running state. If the VM has not yet been launched
/// (by sending a Propolis instance-ensure request to it), send that request
/// first.
Expand Down
Loading

0 comments on commit 7d94141

Please sign in to comment.