Block size must be given during disk create

common/src/api/external/mod.rs

@@ -755,7 +755,9 @@ pub struct Disk {
     pub identity: IdentityMetadata,
     pub project_id: Uuid,
     pub snapshot_id: Option<Uuid>,
+    pub image_id: Option<Uuid>,
     pub size: ByteCount,
+    pub block_size: ByteCount,
     pub state: DiskState,
     pub device_path: String,
 }

common/src/sql/dbinit.sql

@@ -365,6 +365,12 @@ CREATE UNIQUE INDEX ON omicron.public.instance (
 --     'faulted'
 -- );
 
+CREATE TYPE omicron.public.block_size AS ENUM (
+  '512',
+  '2048',
+  '4096'
+);
+
 CREATE TABLE omicron.public.disk (
     /* Identity metadata (resource) */
     id UUID PRIMARY KEY,
@@ -400,7 +406,9 @@ CREATE TABLE omicron.public.disk (
 
     /* Disk configuration */
     size_bytes INT NOT NULL,
-    origin_snapshot UUID
+    block_size omicron.public.block_size NOT NULL,
+    origin_snapshot UUID,
+    origin_image UUID
 );
 
 CREATE UNIQUE INDEX ON omicron.public.disk (

nexus/src/db/datastore.rs

@@ -3052,7 +3052,9 @@ mod test {
                 description: name.to_string(),
             },
             snapshot_id: None,
+            image_id: None,
             size,
+            block_size: params::BlockSize::try_from(4096).unwrap(),
         }
     }
 

nexus/src/db/model.rs

@@ -120,9 +120,9 @@ macro_rules! impl_enum_type {
 
         $(#[$model_meta:meta])*
         pub enum $model_type:ident;
+
         $($enum_item:ident => $sql_value:literal)+
     ) => {
-
         $(#[$enum_meta])*
         pub struct $diesel_type;
 
@@ -1281,6 +1281,49 @@ impl From<InstanceState> for sled_agent_client::types::InstanceState {
     }
 }
 
+impl_enum_type!(
+    #[derive(SqlType, Debug, QueryId)]
+    #[postgres(type_name = "block_size", type_schema = "public")]
+    pub struct BlockSizeEnum;
+
+    #[derive(Copy, Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize, PartialEq)]
+    #[sql_type = "BlockSizeEnum"]
+    pub enum BlockSize;
+
+    // Enum values
+    Traditional => b"512"
+    Iso => b"2048"
+    AdvancedFormat => b"4096"
+);
+
+impl BlockSize {
+    pub fn to_bytes(&self) -> u32 {
+        match self {
+            BlockSize::Traditional => 512,
+            BlockSize::Iso => 2048,
+            BlockSize::AdvancedFormat => 4096,
+        }
+    }
+}
+
+impl Into<external::ByteCount> for BlockSize {
+    fn into(self) -> external::ByteCount {
+        external::ByteCount::from(self.to_bytes())
+    }
+}
+
+impl TryFrom<params::BlockSize> for BlockSize {
+    type Error = anyhow::Error;
+    fn try_from(block_size: params::BlockSize) -> Result<Self, Self::Error> {
+        match block_size.0 {
+            512 => Ok(BlockSize::Traditional),
+            2048 => Ok(BlockSize::Iso),
+            4096 => Ok(BlockSize::AdvancedFormat),
+            _ => anyhow::bail!("invalid block size {}", block_size.0),
+        }
+    }
+}
+
 /// A Disk (network block device).
 #[derive(
     Queryable,
@@ -1313,10 +1356,19 @@ pub struct Disk {
     /// size of the Disk
     #[column_name = "size_bytes"]
     pub size: ByteCount,
+
+    /// size of blocks (512, 2048, or 4096)
+    pub block_size: BlockSize,
+
     /// id for the snapshot from which this Disk was created (None means a blank
     /// disk)
     #[column_name = "origin_snapshot"]
     pub create_snapshot_id: Option<Uuid>,
+
+    /// id for the image from which this Disk was created (None means a blank
+    /// disk)
+    #[column_name = "origin_image"]
+    pub create_image_id: Option<Uuid>,
 }
 
 impl Disk {
@@ -1326,17 +1378,19 @@ impl Disk {
         volume_id: Uuid,
         params: params::DiskCreate,
         runtime_initial: DiskRuntimeState,
-    ) -> Self {
+    ) -> Result<Self, anyhow::Error> {
         let identity = DiskIdentity::new(disk_id, params.identity);
-        Self {
+        Ok(Self {
             identity,
             rcgen: external::Generation::new().into(),
             project_id,
             volume_id,
             runtime_state: runtime_initial,
             size: params.size.into(),
+            block_size: params.block_size.try_into()?,
             create_snapshot_id: params.snapshot_id,
-        }
+            create_image_id: params.image_id,
+        })
     }
 
     pub fn state(&self) -> DiskState {
@@ -1360,7 +1414,9 @@ impl Into<external::Disk> for Disk {
             identity: self.identity(),
             project_id: self.project_id,
             snapshot_id: self.create_snapshot_id,
+            image_id: self.create_image_id,
             size: self.size.into(),
+            block_size: self.block_size.into(),
             state: self.state().into(),
             device_path,
         }

nexus/src/db/schema.rs

@@ -22,7 +22,9 @@ table! {
         state_generation -> Int8,
         time_state_updated -> Timestamptz,
         size_bytes -> Int8,
+        block_size -> crate::db::model::BlockSizeEnum,
         origin_snapshot -> Nullable<Uuid>,
+        origin_image -> Nullable<Uuid>,
     }
 }
 

nexus/src/external_api/params.rs

@@ -10,6 +10,7 @@ use omicron_common::api::external::{
 };
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
+use std::collections::BTreeMap;
 use std::net::IpAddr;
 use uuid::Uuid;
 
@@ -229,28 +230,91 @@ pub struct VpcRouterUpdate {
 
 // DISKS
 
+#[derive(Copy, Clone, Debug, Deserialize, Serialize)]
+pub struct BlockSize(pub u32);
+
+impl TryFrom<u32> for BlockSize {
+    type Error = anyhow::Error;
+    fn try_from(x: u32) -> Result<BlockSize, Self::Error> {
+        if ![512, 2048, 4096].contains(&x) {
+            anyhow::bail!("invalid block size {}", x);
+        }
+
+        Ok(BlockSize(x))
+    }
+}
+
+impl Into<ByteCount> for BlockSize {
+    fn into(self) -> ByteCount {
+        ByteCount::from(self.0)
+    }
+}
+
+impl JsonSchema for BlockSize {
+    fn schema_name() -> String {
+        "BlockSize".to_string()
+    }
+
+    fn json_schema(
+        _gen: &mut schemars::gen::SchemaGenerator,
+    ) -> schemars::schema::Schema {
+        schemars::schema::Schema::Object(schemars::schema::SchemaObject {
+            metadata: Some(Box::new(schemars::schema::Metadata {
+                id: None,
+                title: Some("disk block size in bytes".to_string()),
+                description: None,
+                default: None,
+                deprecated: false,
+                read_only: false,
+                write_only: false,
+                examples: vec![],
+            })),
+            instance_type: Some(schemars::schema::SingleOrVec::Single(
+                Box::new(schemars::schema::InstanceType::Integer),
+            )),
+            format: None,
+            enum_values: Some(vec![
+                serde_json::json!(512),
+                serde_json::json!(2048),
+                serde_json::json!(4096),
+            ]),
+            const_value: None,
+            subschemas: None,
+            number: None,
+            string: None,
+            array: None,
+            object: None,
+            reference: None,
+            extensions: BTreeMap::new(),
+        })
+    }
+}
+
 /// Create-time parameters for a [`Disk`](omicron_common::api::external::Disk)
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
 pub struct DiskCreate {
     /// common identifying metadata
     #[serde(flatten)]
     pub identity: IdentityMetadataCreateParams,
     /// id for snapshot from which the Disk should be created, if any
-    pub snapshot_id: Option<Uuid>, // TODO should be a name?
-    /// size of the Disk
+    pub snapshot_id: Option<Uuid>,
+    /// id for image from which the Disk should be created, if any
+    pub image_id: Option<Uuid>,
+    /// total size of the Disk in bytes
     pub size: ByteCount,
+    /// size of blocks for this Disk. valid values are: 512, 2048, or 4096
+    pub block_size: BlockSize,
 }
 
-const BLOCK_SIZE: u32 = 1_u32 << 12;
 const EXTENT_SIZE: u32 = 1_u32 << 20;
 
 impl DiskCreate {
     pub fn block_size(&self) -> ByteCount {
-        ByteCount::from(BLOCK_SIZE)
+        ByteCount::from(self.block_size.0)
     }
 
     pub fn blocks_per_extent(&self) -> i64 {
-        EXTENT_SIZE as i64 / BLOCK_SIZE as i64
+        EXTENT_SIZE as i64 / i64::from(self.block_size.0)
     }
 
     pub fn extent_count(&self) -> i64 {
@@ -335,7 +399,9 @@ mod test {
                 description: "desc".to_string(),
             },
             snapshot_id: None,
+            image_id: None,
             size,
+            block_size: BlockSize::try_from(4096).unwrap(),
         }
     }
 

nexus/src/nexus.rs

@@ -773,15 +773,35 @@ impl Nexus {
             .lookup_for(authz::Action::CreateChild)
             .await?;
 
-        // Until we implement snapshots, do not allow disks to be created with a
-        // snapshot id.
+        // Reject disks where the block size doesn't evenly divide the total
+        // size
+        if (params.size.to_bytes() % params.block_size().to_bytes()) != 0 {
+            return Err(Error::InvalidValue {
+                label: String::from("size and block_size"),
+                message: String::from(
+                    "total size must be a multiple of block size",
+                ),
+            });
+        }
+
+        // Until we implement snapshots, do not allow disks to be created from a
+        // snapshot.
         if params.snapshot_id.is_some() {
             return Err(Error::InvalidValue {
                 label: String::from("snapshot_id"),
                 message: String::from("snapshots are not yet supported"),
             });
         }
 
+        // Until we implement images, do not allow disks to be created from an
+        // image.
+        if params.image_id.is_some() {
+            return Err(Error::InvalidValue {
+                label: String::from("image_id"),
+                message: String::from("images are not yet supported"),
+            });
+        }
+
         let saga_params = Arc::new(sagas::ParamsDiskCreate {
             serialized_authn: authn::saga::Serialized::for_opctx(opctx),
             project_id: authz_project.id(),

nexus/src/sagas.rs

@@ -1019,18 +1019,24 @@ async fn sdc_create_disk_record(
     // but this should be acceptable because the disk remains in a "Creating"
     // state until the saga has completed.
     let volume_id = sagactx.lookup::<Uuid>("volume_id")?;
+
     let disk = db::model::Disk::new(
         disk_id,
         params.project_id,
         volume_id,
         params.create_params.clone(),
         db::model::DiskRuntimeState::new(),
-    );
+    )
+    .map_err(|e| {
+        ActionError::action_failed(Error::invalid_request(&e.to_string()))
+    })?;
+
     let disk_created = osagactx
         .datastore()
         .project_create_disk(disk)
         .await
         .map_err(ActionError::action_failed)?;
+
     Ok(disk_created)
 }
 

nexus/test-utils/src/resource_helpers.rs

@@ -133,7 +133,9 @@ pub async fn create_disk(
                 description: String::from("sells rainsticks"),
             },
             snapshot_id: None,
+            image_id: None,
             size: ByteCount::from_gibibytes_u32(1),
+            block_size: params::BlockSize::try_from(512).unwrap(),
         },
     )
     .await