New downstairs clone subcommand.

common/src/lib.rs

@@ -158,6 +158,9 @@ pub enum CrucibleError {
 
     #[error("missing block context for non-empty block")]
     MissingBlockContext,
+
+    #[error("Incompatible RegionDefinition {0}")]
+    RegionIncompatible(String),
 }
 
 impl From<std::io::Error> for CrucibleError {
@@ -363,6 +366,7 @@ impl From<CrucibleError> for dropshot::HttpError {
             | CrucibleError::ModifyingReadOnlyRegion
             | CrucibleError::OffsetInvalid
             | CrucibleError::OffsetUnaligned
+            | CrucibleError::RegionIncompatible(_)
             | CrucibleError::ReplaceRequestInvalid(_)
             | CrucibleError::SnapshotExistsAlready(_)
             | CrucibleError::Unsupported(_) => {

common/src/region.rs

@@ -1,5 +1,6 @@
 // Copyright 2021 Oxide Computer Company
 use anyhow::{bail, Result};
+use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use uuid::Uuid;
 
@@ -17,7 +18,16 @@ use super::*;
  * downstairs expects Block { 2, 12 }.
  */
 #[derive(
-    Deserialize, Serialize, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord,
+    Deserialize,
+    Serialize,
+    Copy,
+    Clone,
+    Debug,
+    PartialEq,
+    Eq,
+    JsonSchema,
+    PartialOrd,
+    Ord,
 )]
 pub struct Block {
     // Value could mean a size or offset
@@ -118,7 +128,7 @@ impl Block {
     }
 }
 
-#[derive(Deserialize, Serialize, Copy, Clone, Debug, PartialEq)]
+#[derive(Deserialize, Serialize, Copy, Clone, Debug, JsonSchema, PartialEq)]
 pub struct RegionDefinition {
     /**
      * The size of each block in bytes. Must be a power of 2, minimum 512.
@@ -170,6 +180,55 @@ impl RegionDefinition {
         })
     }
 
+    // Compare two RegionDefinitions and verify they are compatible.
+    // compatible is valid if all fields are the same, expect for the
+    // UUID. The UUID should be different.
+    pub fn compatible(
+        self,
+        other: RegionDefinition,
+    ) -> Result<(), CrucibleError> {
+        // These fields should be the same.
+        if self.block_size != other.block_size {
+            return Err(CrucibleError::RegionIncompatible(
+                "block_size".to_string(),
+            ));
+        }
+        if self.extent_size != other.extent_size {
+            return Err(CrucibleError::RegionIncompatible(
+                "extent_size".to_string(),
+            ));
+        }
+        if self.extent_count != other.extent_count {
+            return Err(CrucibleError::RegionIncompatible(
+                "extent_count".to_string(),
+            ));
+        }
+        if self.encrypted != other.encrypted {
+            return Err(CrucibleError::RegionIncompatible(
+                "encrypted".to_string(),
+            ));
+        }
+        if self.database_read_version != other.database_read_version {
+            return Err(CrucibleError::RegionIncompatible(
+                "database_read_version".to_string(),
+            ));
+        }
+        if self.database_write_version != other.database_write_version {
+            return Err(CrucibleError::RegionIncompatible(
+                "database_write_version".to_string(),
+            ));
+        }
+
+        // If the UUIDs are the same, this is invalid.
+        if self.uuid == other.uuid {
+            return Err(CrucibleError::RegionIncompatible(
+                "UUIDs are the same".to_string(),
+            ));
+        }
+
+        Ok(())
+    }
+
     pub fn database_read_version(&self) -> usize {
         self.database_read_version
     }
@@ -489,4 +548,113 @@ mod test {
          */
         assert!(rd.validate_io(Block::new(1, 9), 2048).is_err());
     }
+
+    fn test_rd() -> RegionDefinition {
+        RegionDefinition {
+            block_size: 512,
+            extent_size: Block::new(10, 9),
+            extent_count: 8,
+            uuid: Uuid::new_v4(),
+            encrypted: false,
+            database_read_version: DATABASE_READ_VERSION,
+            database_write_version: DATABASE_WRITE_VERSION,
+        }
+    }
+
+    #[test]
+    fn test_region_compare_block() {
+        let mut rd1 = test_rd();
+        let rd2 = test_rd();
+
+        // Basic positive test first.
+        assert_eq!(rd1.compatible(rd2), Ok(()));
+
+        rd1.block_size = 4096;
+        assert!(rd1.compatible(rd2).is_err());
+
+        let rd1 = test_rd();
+        let mut rd2 = test_rd();
+        rd2.block_size = 4096;
+        assert!(rd1.compatible(rd2).is_err());
+    }
+
+    #[test]
+    fn test_region_compare_extent_size() {
+        let mut rd1 = test_rd();
+        let rd2 = test_rd();
+
+        rd1.extent_size = Block::new(2, 9);
+        assert!(rd1.compatible(rd2).is_err());
+
+        let rd1 = test_rd();
+        let mut rd2 = test_rd();
+        rd2.extent_size = Block::new(2, 9);
+        assert!(rd1.compatible(rd2).is_err());
+    }
+
+    #[test]
+    fn test_region_compare_extent_count() {
+        let mut rd1 = test_rd();
+        let rd2 = test_rd();
+
+        rd1.extent_count = 9;
+        assert!(rd1.compatible(rd2).is_err());
+
+        let rd1 = test_rd();
+        let mut rd2 = test_rd();
+        rd2.extent_count = 9;
+        assert!(rd1.compatible(rd2).is_err());
+    }
+
+    #[test]
+    fn test_region_compare_uuid() {
+        // Verify region compare, UUIDs must be different
+        let mut rd1 = test_rd();
+        let rd2 = test_rd();
+
+        rd1.uuid = rd2.uuid;
+        assert!(rd1.compatible(rd2).is_err());
+    }
+
+    #[test]
+    fn test_region_compare_encrypted() {
+        let mut rd1 = test_rd();
+        let rd2 = test_rd();
+
+        rd1.encrypted = true;
+        assert!(rd1.compatible(rd2).is_err());
+
+        let rd1 = test_rd();
+        let mut rd2 = test_rd();
+        rd2.encrypted = true;
+        assert!(rd1.compatible(rd2).is_err());
+    }
+
+    #[test]
+    fn test_region_compare_db_read_version() {
+        let mut rd1 = test_rd();
+        let rd2 = test_rd();
+
+        rd1.database_read_version = DATABASE_READ_VERSION + 1;
+        assert!(rd1.compatible(rd2).is_err());
+
+        let rd1 = test_rd();
+        let mut rd2 = test_rd();
+        rd2.database_read_version = DATABASE_READ_VERSION + 1;
+        assert!(rd1.compatible(rd2).is_err());
+    }
+
+    #[test]
+    fn test_region_compare_db_write_version() {
+        let mut rd1 = test_rd();
+        let rd2 = test_rd();
+
+        rd1.database_write_version = DATABASE_WRITE_VERSION + 1;
+        assert!(rd1.compatible(rd2).is_err());
+
+        let rd1 = test_rd();
+        let mut rd2 = test_rd();
+        rd2.database_write_version = DATABASE_WRITE_VERSION + 1;
+        assert!(rd1.compatible(rd2).is_err());
+    }
 }

downstairs/src/extent.rs

@@ -144,7 +144,7 @@ impl ExtentMeta {
     }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Copy, Clone)]
 #[allow(clippy::enum_variant_names)]
 pub enum ExtentType {
     Data,
@@ -169,7 +169,7 @@ impl fmt::Display for ExtentType {
  * FileType from the repair client.
  */
 impl ExtentType {
-    pub fn to_file_type(&self) -> FileType {
+    pub fn to_file_type(self) -> FileType {
         match self {
             ExtentType::Data => FileType::Data,
             ExtentType::Db => FileType::Db,
@@ -308,7 +308,7 @@ impl Extent {
                 "Extent {} found replacement dir, finishing replacement",
                 number
             );
-            move_replacement_extent(dir, number as usize, log)?;
+            move_replacement_extent(dir, number as usize, log, false)?;
         }
 
         // We will migrate every read-write extent with a SQLite file present.
@@ -618,6 +618,7 @@ pub(crate) fn move_replacement_extent<P: AsRef<Path>>(
     region_dir: P,
     eid: usize,
     log: &Logger,
+    clone: bool,
 ) -> Result<(), CrucibleError> {
     let destination_dir = extent_dir(&region_dir, eid as u32);
     let extent_file_name = extent_file_name(eid as u32, ExtentType::Data);
@@ -653,13 +654,31 @@ pub(crate) fn move_replacement_extent<P: AsRef<Path>>(
     sync_path(&original_file, log)?;
 
     // We distinguish between SQLite-backend and raw-file extents based on the
-    // presence of the `.db` file.  We should never do live migration across
-    // different extent formats; in fact, we should never live-migrate
-    // SQLite-backed extents at all, but must still handle the case of
-    // unfinished migrations.
+    // presence of the `.db` file.  We should never do extent repair across
+    // different extent formats; it must be SQLite-to-SQLite or raw-to-raw.
+    //
+    // It is uncommon to perform extent repair on SQLite-backed extents at all,
+    // because they are automatically migrated into raw file extents or
+    // read-only.  However, it must be supported for two cases:
+    //
+    // - If there was an unfinished replacement, we must finish that replacement
+    //   before migrating from SQLite -> raw file backend, which happens
+    //   automatically later in startup.
+    // - We use this same code path to perform clones of read-only regions,
+    //   which may be SQLite-backed (and will never migrate to raw files,
+    //   because they are read only).  This is only the case when the `clone`
+    //   argument is `true`.
+    //
+    // In the first case, we know that we are repairing an SQLite-based extent
+    // because the target (original) extent includes a `.db` file.
+    //
+    // In the second case, the target (original) extent is not present, so we
+    // check whether the new files include a `.db` file.
+
     new_file.set_extension("db");
     original_file.set_extension("db");
-    if original_file.exists() {
+
+    if original_file.exists() || (new_file.exists() && clone) {
         if let Err(e) = std::fs::copy(new_file.clone(), original_file.clone()) {
             crucible_bail!(
                 IoError,
@@ -690,6 +709,10 @@ pub(crate) fn move_replacement_extent<P: AsRef<Path>>(
             }
             sync_path(&original_file, log)?;
         } else if original_file.exists() {
+            // If we are cloning, then our new region will have been
+            // created with Backend::RawFile, and we should have no SQLite
+            // files.
+            assert!(!clone);
             info!(
                 log,
                 "Remove old file {:?} as there is no replacement",
@@ -714,6 +737,10 @@ pub(crate) fn move_replacement_extent<P: AsRef<Path>>(
             }
             sync_path(&original_file, log)?;
         } else if original_file.exists() {
+            // If we are cloning, then our new region will have been
+            // created with Backend::RawFile, and we should have no SQLite
+            // files.
+            assert!(!clone);
             info!(
                 log,
                 "Remove old file {:?} as there is no replacement",