New downstairs clone subcommand. (#1129)

New clone subcommand for crucible-downstairs.

This enables a downstairs to "clone" another downstairs using the
repair endpoint.  It requires the source downstairs to be read only
and will destroy whatever data exists on the destination downstairs.

Support for cloning SQLite backend downstairs is supported, and I had
to bring back some of the old repair code that supported the additional
files for that.

Found an fixed a bug where the incorrect file types were returned
during a repair that contained SQLite files.

A bunch more tests were added to cover the clone process, and new
API endpoints were added to the downstairs repair server.

Author: leftwo

Cargo.lock

@@ -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/lib.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());
+    }
 }

common/src/region.rs

@@ -143,7 +143,7 @@ impl ExtentMeta {
     }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Copy, Clone)]
 #[allow(clippy::enum_variant_names)]
 pub enum ExtentType {
     Data,
@@ -168,7 +168,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,
@@ -307,7 +307,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.
@@ -616,6 +616,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);
@@ -651,13 +652,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,
@@ -688,6 +707,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",
@@ -712,6 +735,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",