Merge #1512

1512: feat(pool): blobstore cluster size to be set during pool creation through grpc parameter value r=hrudaya21 a=hrudaya21 - The default cluster size considered by SPDK is 4MiB. - During pool creation, cluster_size can be taken from the gRPC and consider that value. - The cluster_size can be consider with multiple of 4MiB. - If the cluster_size passed not multiple of 1MiB, then default 4MiB cluster size will be considered for the pool creation. Co-authored-by: Hrudaya <[email protected]>
openebs · Oct 4, 2023 · 9e42b76 · 9e42b76
2 parents e89a219 + bdba0dc
commit 9e42b76
Show file tree

Hide file tree

Showing 19 changed files with 132 additions and 26 deletions.
diff --git a/io-engine-tests/src/pool.rs b/io-engine-tests/src/pool.rs
@@ -78,6 +78,7 @@ impl PoolBuilder {
                 uuid: Some(self.uuid()),
                 pooltype: 0,
                 disks: vec![self.bdev.as_ref().unwrap().clone()],
+                cluster_size: None,
             })
             .await
             .map(|r| r.into_inner())

diff --git a/io-engine/src/bdev/lvs.rs b/io-engine/src/bdev/lvs.rs
@@ -214,6 +214,7 @@ impl Lvs {
             name: self.name.to_owned(),
             disks: vec![self.disk.to_owned()],
             uuid: None,
+            cluster_size: None,
         };
         match &self.mode {
             LvsMode::Create => {

diff --git a/io-engine/src/bin/io-engine-client/v1/pool_cli.rs b/io-engine/src/bin/io-engine-client/v1/pool_cli.rs
@@ -1,5 +1,6 @@
 use crate::{
     context::{Context, OutputFormat},
+    parse_size,
     ClientError,
     GrpcStatus,
 };
@@ -26,6 +27,13 @@ pub fn subcommands<'a, 'b>() -> App<'a, 'b> {
                 .takes_value(true)
                 .help("Storage pool uuid"),
         )
+        .arg(
+            Arg::with_name("cluster-size")
+                .long("cluster-size")
+                .required(false)
+                .takes_value(true)
+                .help("SPDK cluster size"),
+        )
         .arg(
             Arg::with_name("disk")
                 .required(true)
@@ -124,7 +132,10 @@ async fn create(
         })?
         .map(|dev| dev.to_owned())
         .collect();
-
+    let cluster_size =
+        parse_size(matches.value_of("cluster-size").unwrap_or_default())
+            .map_err(|s| Status::invalid_argument(format!("Bad size '{s}'")))
+            .context(GrpcStatus)?;
     let response = ctx
         .v1
         .pool
@@ -133,6 +144,7 @@ async fn create(
             uuid: uuid.map(ToString::to_string),
             disks: disks_list,
             pooltype: v1rpc::pool::PoolType::Lvs as i32,
+            cluster_size: Some(cluster_size.get_bytes() as u32),
         })
         .await
         .context(GrpcStatus)?;

diff --git a/io-engine/src/grpc/v0/mayastor_grpc.rs b/io-engine/src/grpc/v0/mayastor_grpc.rs
@@ -209,6 +209,7 @@ impl TryFrom<CreatePoolRequest> for PoolArgs {
                 name: args.name,
                 disks: args.disks,
                 uuid: None,
+                cluster_size: None,
             }),
         }
     }

diff --git a/io-engine/src/grpc/v1/pool.rs b/io-engine/src/grpc/v1/pool.rs
@@ -89,6 +89,7 @@ impl TryFrom<CreatePoolRequest> for PoolArgs {
             name: args.name,
             disks: args.disks,
             uuid: args.uuid,
+            cluster_size: args.cluster_size,
         })
     }
 }
@@ -116,6 +117,7 @@ impl TryFrom<ImportPoolRequest> for PoolArgs {
             name: args.name,
             disks: args.disks,
             uuid: args.uuid,
+            cluster_size: None,
         })
     }
 }
@@ -149,6 +151,7 @@ impl From<Lvs> for Pool {
             used: l.used(),
             committed: l.committed(),
             pooltype: PoolType::Lvs as i32,
+            cluster_size: l.blob_cluster_size() as u32,
         }
     }
 }

diff --git a/io-engine/src/lvs/lvol_snapshot.rs b/io-engine/src/lvs/lvol_snapshot.rs
@@ -500,7 +500,6 @@ impl SnapshotOps for Lvol {
             }
             s.send((errno, lvol_ptr)).ok();
         }
-
         let (s, r) = oneshot::channel::<(i32, *mut spdk_lvol)>();
 
         self.do_create_snapshot(

diff --git a/io-engine/src/lvs/lvs_error.rs b/io-engine/src/lvs/lvs_error.rs
@@ -59,6 +59,17 @@ pub enum Error {
         source: Errno,
         msg: String,
     },
+    #[snafu(display(
+        "errno {}: Invalid cluster-size {}, for pool {}",
+        source,
+        msg,
+        name
+    ))]
+    InvalidClusterSize {
+        source: Errno,
+        name: String,
+        msg: String,
+    },
     #[snafu(display("lvol exists {}", name))]
     RepExists {
         source: Errno,
@@ -195,6 +206,9 @@ impl ToErrno for Error {
             Self::Invalid {
                 source, ..
             } => source,
+            Self::InvalidClusterSize {
+                source, ..
+            } => source,
             Self::RepExists {
                 source, ..
             } => source,

diff --git a/io-engine/src/lvs/lvs_store.rs b/io-engine/src/lvs/lvs_store.rs
@@ -56,6 +56,12 @@ use events_api::event::EventAction;
 
 use crate::eventing::Event;
 
+static ROUND_TO_MB: u32 = 1024 * 1024;
+/// Default spdk cluster size is 4MiB.
+static DEFAULT_CLUSTER_SIZE: u32 = 4 * 1024 * 1024;
+/// Maximum spdk cluster size can be considered as 1GiB.
+static MAX_CLUSTER_SIZE: u32 = 1024 * 1024 * 1024;
+
 impl Debug for Lvs {
     fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
         write!(
@@ -399,10 +405,32 @@ impl Lvs {
         name: &str,
         bdev: &str,
         uuid: Option<String>,
+        cluster_size: Option<u32>,
     ) -> Result<Lvs, Error> {
         let pool_name = name.into_cstring();
         let bdev_name = bdev.into_cstring();
-
+        let cluster_size = if let Some(cluster_size) = cluster_size {
+            if cluster_size % ROUND_TO_MB == 0 {
+                cluster_size
+            } else {
+                return Err(Error::InvalidClusterSize {
+                    source: Errno::EINVAL,
+                    name: name.to_string(),
+                    msg: format!("{cluster_size}, not multiple of 1MiB"),
+                });
+            }
+        } else {
+            DEFAULT_CLUSTER_SIZE
+        };
+        if cluster_size > MAX_CLUSTER_SIZE {
+            return Err(Error::InvalidClusterSize {
+                source: Errno::EINVAL,
+                name: name.to_string(),
+                msg: format!(
+                    "{cluster_size}, larger than max limit {MAX_CLUSTER_SIZE}"
+                ),
+            });
+        }
         let (sender, receiver) = pair::<ErrnoResult<Lvs>>();
         unsafe {
             if let Some(uuid) = uuid {
@@ -411,7 +439,7 @@ impl Lvs {
                     bdev_name.as_ptr(),
                     pool_name.as_ptr(),
                     cuuid.as_ptr(),
-                    0,
+                    cluster_size,
                     // We used to clear a pool with UNMAP but that takes
                     // awfully long time on large SSDs (~
                     // can take an hour). Clearing the pool
@@ -427,7 +455,7 @@ impl Lvs {
                 vbdev_lvs_create(
                     bdev_name.as_ptr(),
                     pool_name.as_ptr(),
-                    0,
+                    cluster_size,
                     // We used to clear a pool with UNMAP but that takes
                     // awfully long time on large SSDs (~
                     // can take an hour). Clearing the pool
@@ -520,7 +548,14 @@ impl Lvs {
             Err(Error::Import {
                 source, ..
             }) if source == Errno::EILSEQ => {
-                match Self::create(&args.name, &bdev, args.uuid).await {
+                match Self::create(
+                    &args.name,
+                    &bdev,
+                    args.uuid,
+                    args.cluster_size,
+                )
+                .await
+                {
                     Err(create) => {
                         let _ = parsed.destroy().await.map_err(|_e| {
                             // we failed to delete the base_bdev be loud about it
@@ -783,7 +818,6 @@ impl Lvs {
         }
 
         let (s, r) = pair::<ErrnoResult<*mut spdk_lvol>>();
-
         let cname = name.into_cstring();
         unsafe {
             match uuid {

diff --git a/io-engine/src/pool_backend.rs b/io-engine/src/pool_backend.rs
@@ -8,6 +8,7 @@ pub struct PoolArgs {
     pub name: String,
     pub disks: Vec<String>,
     pub uuid: Option<String>,
+    pub cluster_size: Option<u32>,
 }
 
 /// PoolBackend is the type of pool underneath Lvs, Lvm, etc

diff --git a/io-engine/src/subsys/config/pool.rs b/io-engine/src/subsys/config/pool.rs
@@ -174,6 +174,7 @@ impl From<&Pool> for PoolArgs {
             name: pool.name.clone(),
             disks: pool.disks.clone(),
             uuid: None,
+            cluster_size: None,
         }
     }
 }

diff --git a/io-engine/tests/lvs_pool.rs b/io-engine/tests/lvs_pool.rs
@@ -42,6 +42,7 @@ async fn lvs_pool_test() {
             name: "tpool".into(),
             disks: vec!["aio:///tmp/disk1.img".into()],
             uuid: None,
+            cluster_size: None,
         })
         .await
         .unwrap();
@@ -53,7 +54,7 @@ async fn lvs_pool_test() {
     // have an idempotent snafu, we dont crash and
     // burn
     ms.spawn(async {
-        assert!(Lvs::create("tpool", "aio:///tmp/disk1.img", None)
+        assert!(Lvs::create("tpool", "aio:///tmp/disk1.img", None, None)
             .await
             .is_err())
     })
@@ -109,7 +110,7 @@ async fn lvs_pool_test() {
         assert!(Lvs::import("tpool", "aio:///tmp/disk1.img").await.is_err());
 
         assert_eq!(Lvs::iter().count(), 0);
-        assert!(Lvs::create("tpool", "aio:///tmp/disk1.img", None)
+        assert!(Lvs::create("tpool", "aio:///tmp/disk1.img", None, None)
             .await
             .is_ok());
 
@@ -138,6 +139,7 @@ async fn lvs_pool_test() {
             name: "tpool2".to_string(),
             disks: vec!["malloc:///malloc0?size_mb=64".to_string()],
             uuid: None,
+            cluster_size: None,
         })
         .await
         .unwrap();
@@ -172,6 +174,7 @@ async fn lvs_pool_test() {
             name: "tpool".to_string(),
             disks: vec!["aio:///tmp/disk1.img".to_string()],
             uuid: None,
+            cluster_size: None,
         })
         .await
         .unwrap();
@@ -304,6 +307,7 @@ async fn lvs_pool_test() {
             name: "tpool".into(),
             disks: vec!["aio:///tmp/disk1.img".into()],
             uuid: None,
+            cluster_size: None,
         })
         .await
         .unwrap();
@@ -333,6 +337,7 @@ async fn lvs_pool_test() {
             name: "jpool".into(),
             disks: vec!["aio:///tmp/disk1.img".into()],
             uuid: None,
+            cluster_size: None,
         })
         .await
         .err()
@@ -348,6 +353,7 @@ async fn lvs_pool_test() {
             name: "tpool2".into(),
             disks: vec!["/tmp/disk2.img".into()],
             uuid: None,
+            cluster_size: None,
         })
         .await
         .unwrap();

diff --git a/io-engine/tests/nexus_child_retire.rs b/io-engine/tests/nexus_child_retire.rs
@@ -558,6 +558,7 @@ async fn init_ms_etcd_test() -> ComposeTest {
                 name: POOL_NAME_0.to_string(),
                 disks: vec![BDEV_NAME_0.to_string()],
                 uuid: None,
+                cluster_size: None,
             })
             .await
             .unwrap();
@@ -572,6 +573,7 @@ async fn init_ms_etcd_test() -> ComposeTest {
                 name: POOL_NAME_1.to_string(),
                 disks: vec![DISK_NAME_1.to_string()],
                 uuid: None,
+                cluster_size: None,
             })
             .await
             .unwrap();

diff --git a/io-engine/tests/nexus_io.rs b/io-engine/tests/nexus_io.rs
@@ -1072,6 +1072,7 @@ async fn nexus_io_write_zeroes() {
                 name: POOL_NAME.to_string(),
                 disks: vec![BDEVNAME1.to_string()],
                 uuid: None,
+                cluster_size: None,
             })
             .await
             .unwrap();

diff --git a/io-engine/tests/nexus_with_local.rs b/io-engine/tests/nexus_with_local.rs
@@ -48,6 +48,7 @@ async fn create_replicas(h: &mut RpcHandle) {
             uuid: Some(pool_uuid()),
             pooltype: 0,
             disks: vec!["malloc:///disk0?size_mb=64".into()],
+            cluster_size: None,
         })
         .await
         .unwrap();

diff --git a/io-engine/tests/nvmf_connect.rs b/io-engine/tests/nvmf_connect.rs
@@ -96,6 +96,7 @@ async fn init_nvmf_share() -> String {
                 name: POOL_NAME.to_string(),
                 disks: vec![BDEV_NAME.to_string()],
                 uuid: None,
+                cluster_size: None,
             })
             .await
             .unwrap();

diff --git a/io-engine/tests/replica_snapshot.rs b/io-engine/tests/replica_snapshot.rs
@@ -96,6 +96,7 @@ async fn replica_snapshot() {
                 name: POOL1_NAME.to_string(),
                 disks: vec![format!("aio://{DISKNAME1}")],
                 uuid: None,
+                cluster_size: None,
             })
             .await
             .unwrap();
-Original file line number
+Diff line change
@@ Expand Up / @@ -500,7 +500,6 @@ impl SnapshotOps for Lvol { @@
                 }
                 s.send((errno, lvol_ptr)).ok();
             }
             let (s, r) = oneshot::channel::<(i32, *mut spdk_lvol)>();
             self.do_create_snapshot(
@@ Expand Down @@