From bdba0dc1b79b4483f9ea3a2598d5a0552a4a4f5d Mon Sep 17 00:00:00 2001 From: Hrudaya Date: Fri, 22 Sep 2023 11:08:29 +0000 Subject: [PATCH] feat(pool): blobstore cluster size to be set during pool creation through grpc parameter value Signed-off-by: Hrudaya --- io-engine-tests/src/pool.rs | 1 + io-engine/src/bdev/lvs.rs | 1 + .../src/bin/io-engine-client/v1/pool_cli.rs | 14 ++++- io-engine/src/grpc/v0/mayastor_grpc.rs | 1 + io-engine/src/grpc/v1/pool.rs | 3 + io-engine/src/lvs/lvol_snapshot.rs | 1 - io-engine/src/lvs/lvs_error.rs | 14 +++++ io-engine/src/lvs/lvs_store.rs | 44 ++++++++++++-- io-engine/src/pool_backend.rs | 1 + io-engine/src/subsys/config/pool.rs | 1 + io-engine/tests/lvs_pool.rs | 10 +++- io-engine/tests/nexus_child_retire.rs | 2 + io-engine/tests/nexus_io.rs | 1 + io-engine/tests/nexus_with_local.rs | 1 + io-engine/tests/nvmf_connect.rs | 1 + io-engine/tests/replica_snapshot.rs | 1 + io-engine/tests/snapshot_lvol.rs | 58 ++++++++++++++----- io-engine/tests/snapshot_nexus.rs | 1 + rpc/mayastor-api | 2 +- 19 files changed, 132 insertions(+), 26 deletions(-) diff --git a/io-engine-tests/src/pool.rs b/io-engine-tests/src/pool.rs index 047e623b53..3d4e1a4f0d 100644 --- 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 index 281673e10a..f347809b26 100644 --- 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 index d529e068c8..af18129c91 100644 --- 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 index ebaf9eab4c..78dd92930c 100644 --- a/io-engine/src/grpc/v0/mayastor_grpc.rs +++ b/io-engine/src/grpc/v0/mayastor_grpc.rs @@ -209,6 +209,7 @@ impl TryFrom 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 index 9c8eea76fa..eadb4250ca 100644 --- a/io-engine/src/grpc/v1/pool.rs +++ b/io-engine/src/grpc/v1/pool.rs @@ -89,6 +89,7 @@ impl TryFrom for PoolArgs { name: args.name, disks: args.disks, uuid: args.uuid, + cluster_size: args.cluster_size, }) } } @@ -116,6 +117,7 @@ impl TryFrom for PoolArgs { name: args.name, disks: args.disks, uuid: args.uuid, + cluster_size: None, }) } } @@ -149,6 +151,7 @@ impl From 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 index 3a506c3052..e8d2b2d0f9 100644 --- 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 index ed62c233f0..e1028c40c0 100644 --- 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 index afb8c5847a..3533ffb1e9 100644 --- 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, + cluster_size: Option, ) -> Result { 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::>(); 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::>(); - let cname = name.into_cstring(); unsafe { match uuid { diff --git a/io-engine/src/pool_backend.rs b/io-engine/src/pool_backend.rs index cf6f8eb5fd..e87e5a31a0 100644 --- 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, pub uuid: Option, + pub cluster_size: Option, } /// 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 index 1e5a1993c9..f0c857abbb 100644 --- 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 index 4877080d9f..d80071ce87 100644 --- 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 index 1bcd811bb4..40d01a6ad1 100644 --- a/io-engine/tests/nexus_child_retire.rs +++ b/io-engine/tests/nexus_child_retire.rs @@ -553,6 +553,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(); @@ -567,6 +568,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 index ddec129e46..a5e82aa726 100644 --- a/io-engine/tests/nexus_io.rs +++ b/io-engine/tests/nexus_io.rs @@ -1067,6 +1067,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 index 146dae3bc4..7f0cb2f6bd 100644 --- 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 index ee37964bf1..ba4d0cf014 100644 --- 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 index 9f18c8eb02..4c09052a78 100644 --- a/io-engine/tests/replica_snapshot.rs +++ b/io-engine/tests/replica_snapshot.rs @@ -93,6 +93,7 @@ async fn replica_snapshot() { name: POOL1_NAME.to_string(), disks: vec![format!("aio://{DISKNAME1}")], uuid: None, + cluster_size: None, }) .await .unwrap(); diff --git a/io-engine/tests/snapshot_lvol.rs b/io-engine/tests/snapshot_lvol.rs index 00c2725336..d1c9e952c8 100755 --- a/io-engine/tests/snapshot_lvol.rs +++ b/io-engine/tests/snapshot_lvol.rs @@ -52,11 +52,16 @@ fn get_ms() -> &'static MayastorTest<'static> { } /// Must be called only in Mayastor context !s -async fn create_test_pool(pool_name: &str, disk: String) -> Lvs { +async fn create_test_pool( + pool_name: &str, + disk: String, + cluster_size: Option, +) -> Lvs { Lvs::create_or_import(PoolArgs { name: pool_name.to_string(), disks: vec![disk], uuid: None, + cluster_size, }) .await .expect("Failed to create test pool"); @@ -150,7 +155,7 @@ async fn test_lvol_alloc_after_snapshot(index: u32, thin: bool) { ms.spawn(async move { // Create a pool and lvol. - let pool = create_test_pool(&pool_name, disk).await; + let pool = create_test_pool(&pool_name, disk, None).await; let cluster_size = pool.blob_cluster_size(); let lvol = pool .create_lvol( @@ -288,9 +293,12 @@ async fn test_lvol_bdev_snapshot() { ms.spawn(async move { // Create a pool and lvol. - let pool = - create_test_pool("pool1", "malloc:///disk0?size_mb=64".to_string()) - .await; + let pool = create_test_pool( + "pool1", + "malloc:///disk0?size_mb=64".to_string(), + None, + ) + .await; let lvol = pool .create_lvol( "lvol1", @@ -342,9 +350,12 @@ async fn test_lvol_handle_snapshot() { ms.spawn(async move { // Create a pool and lvol. - let pool = - create_test_pool("pool2", "malloc:///disk1?size_mb=64".to_string()) - .await; + let pool = create_test_pool( + "pool2", + "malloc:///disk1?size_mb=64".to_string(), + None, + ) + .await; pool.create_lvol( "lvol2", @@ -396,9 +407,12 @@ async fn test_lvol_list_snapshot() { ms.spawn(async move { // Create a pool and lvol. - let pool = - create_test_pool("pool3", "malloc:///disk3?size_mb=64".to_string()) - .await; + let pool = create_test_pool( + "pool3", + "malloc:///disk3?size_mb=64".to_string(), + None, + ) + .await; let lvol = pool .create_lvol( "lvol3", @@ -484,6 +498,7 @@ async fn test_list_all_snapshots() { let pool = create_test_pool( "pool4", "malloc:///disk4?size_mb=128".to_string(), + None, ) .await; let lvol = pool @@ -605,9 +620,12 @@ async fn test_list_pool_snapshots() { ms.spawn(async move { // Create a pool and lvol. - let pool = - create_test_pool("pool6", "malloc:///disk6?size_mb=32".to_string()) - .await; + let pool = create_test_pool( + "pool6", + "malloc:///disk6?size_mb=32".to_string(), + None, + ) + .await; let lvol = pool .create_lvol( @@ -694,6 +712,7 @@ async fn test_list_all_snapshots_with_replica_destroy() { let pool = create_test_pool( "pool7", "malloc:///disk7?size_mb=128".to_string(), + None, ) .await; let lvol = pool @@ -746,11 +765,12 @@ async fn test_snapshot_referenced_size() { let pool = create_test_pool( "pool8", "malloc:///disk8?size_mb=64".to_string(), + Some(1024 * 1024), ) .await; let cluster_size = pool.blob_cluster_size(); - + assert_eq!(cluster_size, 1024 * 1024, "Create cluster size doesn't match with blob cluster size"); let lvol = pool .create_lvol( LVOL_NAME, @@ -971,6 +991,7 @@ async fn test_snapshot_clone() { let pool = create_test_pool( "pool9", "malloc:///disk5?size_mb=128".to_string(), + None, ) .await; let lvol = pool @@ -1084,6 +1105,7 @@ async fn test_snapshot_volume_provisioning_mode() { let pool = create_test_pool( "pool10", "malloc:///disk10?size_mb=64".to_string(), + None, ) .await; @@ -1142,7 +1164,7 @@ async fn test_snapshot_attr() { ms.spawn(async move { // Create a pool and lvol. let mut pool = - create_test_pool("pool20", POOL_DEVICE_NAME.into()).await; + create_test_pool("pool20", POOL_DEVICE_NAME.into(), None).await; let lvol = pool .create_lvol( "lvol20", @@ -1260,6 +1282,7 @@ async fn test_delete_snapshot_with_valid_clone() { let pool = create_test_pool( "pool13", "malloc:///disk13?size_mb=128".to_string(), + None, ) .await; let lvol = pool @@ -1383,6 +1406,7 @@ async fn test_delete_snapshot_with_valid_clone_fail_1() { let pool = create_test_pool( "pool14", "malloc:///disk14?size_mb=128".to_string(), + None, ) .await; let lvol = pool @@ -1567,6 +1591,7 @@ async fn test_snapshot_parent_usage_post_snapshot_destroy() { let pool = create_test_pool( "pool16", "malloc:///disk16?size_mb=128".to_string(), + None, ) .await; let lvol = pool @@ -1651,6 +1676,7 @@ async fn test_clone_snapshot_usage_post_clone_destroy() { let pool = create_test_pool( "pool17", "malloc:///disk17?size_mb=128".to_string(), + None, ) .await; let lvol = pool diff --git a/io-engine/tests/snapshot_nexus.rs b/io-engine/tests/snapshot_nexus.rs index 7f08949f61..8051531e7b 100755 --- a/io-engine/tests/snapshot_nexus.rs +++ b/io-engine/tests/snapshot_nexus.rs @@ -135,6 +135,7 @@ async fn launch_instance(create_replicas: bool) -> (ComposeTest, Vec) { uuid: Some(pool_uuid()), pooltype: 0, disks: vec!["malloc:///disk0?size_mb=128".into()], + cluster_size: None, }) .await .unwrap(); diff --git a/rpc/mayastor-api b/rpc/mayastor-api index a8969cddfb..3365ab90db 160000 --- a/rpc/mayastor-api +++ b/rpc/mayastor-api @@ -1 +1 @@ -Subproject commit a8969cddfbdc0b39b942033a097b0f079f825d53 +Subproject commit 3365ab90db1bcf6b176e8277d9230ca18f0af29f