PHD: Use ZFS clones for file-backed disks

.github/buildomat/jobs/phd-run.sh

@@ -21,6 +21,11 @@ indir="/input"
 indir_suffix="phd-build/out/*.tar.gz"
 phddir="$PWD/phd-test"
 
+# Put artifacts on the runner's SSDs (the /work ramdisk is small by design, too
+# small for images of any appreciable size).
+pfexec zpool create -f phd-artifacts c1t1d0 c2t1d0
+artifactdir="/phd-artifacts"
+
 banner 'Inputs'
 find $indir -ls
 
@@ -63,7 +68,7 @@ set +e
 	--crucible-downstairs-commit auto \
 	--artifact-toml-path $artifacts \
 	--tmp-directory $tmpdir \
-	--artifact-directory $tmpdir | \
+	--artifact-directory $artifactdir | \
 	tee /tmp/phd-runner.log)
 failcount=$?
 set -e

phd-tests/framework/src/disk/file.rs

@@ -4,22 +4,86 @@
 
 //! Abstractions for disks with a raw file backend.
 
-use std::path::{Path, PathBuf};
-
+use camino::{Utf8Path, Utf8PathBuf};
 use propolis_client::types::{FileStorageBackend, StorageBackendV0};
-use tracing::{error, info};
+use tracing::{debug, error, warn};
 use uuid::Uuid;
 
-use crate::guest_os::GuestOsKind;
+use crate::{guest_os::GuestOsKind, zfs::ClonedFile as ZfsClonedFile};
+
+/// Describes the method used to create the backing file for a file-backed disk.
+#[derive(Debug)]
+enum BackingFile {
+    /// The disk is a ZFS clone of the original artifact.
+    Zfs(ZfsClonedFile),
+
+    /// The disk is a hard copy of the original artifact.
+    HardCopy(Utf8PathBuf),
+}
+
+impl BackingFile {
+    /// Creates a new backing file from the artifact with the supplied
+    /// `artifact_path`. If possible, this routine will create a ZFS clone of
+    /// the dataset containing the file; otherwise it will fall back to creating
+    /// a hard copy of the original artifact.
+    fn create_from_source(
+        artifact_path: &Utf8Path,
+        data_dir: &Utf8Path,
+    ) -> anyhow::Result<Self> {
+        match ZfsClonedFile::create_from_path(artifact_path) {
+            Ok(file) => return Ok(Self::Zfs(file)),
+            Err(error) => warn!(
+                %artifact_path,
+                %error,
+                "failed to make ZFS clone of backing artifact, will copy it"
+            ),
+        }
+
+        let mut disk_path = data_dir.to_path_buf();
+        disk_path.push(format!("{}.phd_disk", Uuid::new_v4()));
+        debug!(
+            source = %artifact_path,
+            disk_path = %disk_path,
+            "Copying source image to create temporary disk",
+        );
+
+        std::fs::copy(artifact_path, &disk_path)?;
+        Ok(Self::HardCopy(disk_path))
+    }
+
+    /// Yields the path to this disk's backing file.
+    fn path(&self) -> Utf8PathBuf {
+        match self {
+            BackingFile::Zfs(zfs) => zfs.path(),
+            BackingFile::HardCopy(path) => path.clone(),
+        }
+    }
+}
+
+impl Drop for BackingFile {
+    fn drop(&mut self) {
+        // ZFS clones are cleaned up by their own drop impls.
+        if let BackingFile::HardCopy(path) = self {
+            debug!(%path, "deleting hard copy of guest disk image");
+            if let Err(e) = std::fs::remove_file(&path) {
+                error!(
+                    ?e,
+                    %path,
+                    "failed to delete hard copy of guest disk image"
+                );
+            }
+        }
+    }
+}
 
 /// An RAII wrapper for a disk wrapped by a file.
 #[derive(Debug)]
 pub struct FileBackedDisk {
     /// The name to use for instance spec backends that refer to this disk.
     backend_name: String,
 
-    /// The path at which the disk is stored.
-    disk_path: PathBuf,
+    /// The backing file for this disk.
+    file: BackingFile,
 
     /// The kind of guest OS image this guest contains, or `None` if the disk
     /// was not initialized from a guest OS artifact.
@@ -31,23 +95,18 @@ impl FileBackedDisk {
     /// the specified artifact on the host file system.
     pub(crate) fn new_from_artifact(
         backend_name: String,
-        artifact_path: &impl AsRef<Path>,
-        data_dir: &impl AsRef<Path>,
+        artifact_path: &impl AsRef<Utf8Path>,
+        data_dir: &impl AsRef<Utf8Path>,
         guest_os: Option<GuestOsKind>,
     ) -> Result<Self, super::DiskError> {
-        let mut disk_path = data_dir.as_ref().to_path_buf();
-        disk_path.push(format!("{}.phd_disk", Uuid::new_v4()));
-        info!(
-            source = %artifact_path.as_ref().display(),
-            disk_path = %disk_path.display(),
-            "Copying source image to create temporary disk",
-        );
-
-        std::fs::copy(artifact_path, &disk_path)?;
+        let artifact = BackingFile::create_from_source(
+            artifact_path.as_ref(),
+            data_dir.as_ref(),
+        )?;
 
         // Make sure the disk is writable (the artifact may have been
         // read-only).
-        let disk_file = std::fs::File::open(&disk_path)?;
+        let disk_file = std::fs::File::open(artifact.path())?;
         let mut permissions = disk_file.metadata()?.permissions();
 
         // TODO: Clippy is upset that `set_readonly(false)` results in
@@ -57,7 +116,7 @@ impl FileBackedDisk {
         permissions.set_readonly(false);
         disk_file.set_permissions(permissions)?;
 
-        Ok(Self { backend_name, disk_path, guest_os })
+        Ok(Self { backend_name, file: artifact, guest_os })
     }
 }
 
@@ -66,7 +125,7 @@ impl super::DiskConfig for FileBackedDisk {
         (
             self.backend_name.clone(),
             StorageBackendV0::File(FileStorageBackend {
-                path: self.disk_path.to_string_lossy().to_string(),
+                path: self.file.path().to_string(),
                 readonly: false,
             }),
         )
@@ -76,14 +135,3 @@ impl super::DiskConfig for FileBackedDisk {
         self.guest_os
     }
 }
-
-impl Drop for FileBackedDisk {
-    fn drop(&mut self) {
-        info!(path = %self.disk_path.display(), "Deleting guest disk image");
-        if let Err(e) = std::fs::remove_file(&self.disk_path) {
-            error!(?e,
-                   path = %self.disk_path.display(),
-                   "Failed to delete guest disk image");
-        }
-    }
-}

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

@@ -8,12 +8,10 @@
 //! They can then pass these disks to the VM factory to connect them to a
 //! specific guest VM.
 
-use std::{
-    path::{Path, PathBuf},
-    sync::Arc,
-};
+use std::sync::Arc;
 
 use anyhow::Context;
+use camino::{Utf8Path, Utf8PathBuf};
 use propolis_client::types::StorageBackendV0;
 use thiserror::Error;
 
@@ -108,7 +106,7 @@ pub enum DiskSource<'a> {
 ///      Propolis backend implementations interact with the disk.
 pub(crate) struct DiskFactory {
     /// The directory in which disk files should be stored.
-    storage_dir: PathBuf,
+    storage_dir: Utf8PathBuf,
 
     /// A reference to the artifact store to use to look up guest OS artifacts
     /// when those are used as a disk source.
@@ -127,7 +125,7 @@ impl DiskFactory {
     /// their data in `storage_dir` and will look up guest OS images in the
     /// supplied `artifact_store`.
     pub fn new(
-        storage_dir: &impl AsRef<Path>,
+        storage_dir: &impl AsRef<Utf8Path>,
         artifact_store: Arc<ArtifactStore>,
         port_allocator: Arc<PortAllocator>,
         log_mode: ServerLogMode,
@@ -145,11 +143,10 @@ impl DiskFactory {
     async fn get_guest_artifact_info(
         &self,
         artifact_name: &str,
-    ) -> Result<(PathBuf, GuestOsKind), DiskError> {
+    ) -> Result<(Utf8PathBuf, GuestOsKind), DiskError> {
         self.artifact_store
             .get_guest_os_image(artifact_name)
             .await
-            .map(|(utf8, kind)| (utf8.into_std_path_buf(), kind))
             .with_context(|| {
                 format!("failed to get guest OS artifact '{}'", artifact_name)
             })

phd-tests/framework/src/lib.rs

@@ -55,6 +55,7 @@ mod port_allocator;
 mod serial;
 pub mod server_log_mode;
 pub mod test_vm;
+pub(crate) mod zfs;
 
 /// An instance of the PHD test framework.
 pub struct Framework {