Merge #1903

1903: proto changes for pool expansion r=abhilashshetty04 a=abhilashshetty04



Co-authored-by: Abhilash Shetty <[email protected]>

Author: mayastor-bors

io-engine-tests/src/pool.rs

@@ -212,20 +212,17 @@ impl PoolBuilderRpc {
             .map(|r| r.into_inner())
     }
 
-    pub async fn grow(&mut self) -> Result<(Pool, Pool), Status> {
+    pub async fn grow(&mut self) -> Result<Pool, Status> {
         self.rpc()
             .lock()
             .await
             .pool
-            .grow_pool(GrowPoolRequest {
+            .grow_pool_v2(GrowPoolRequest {
                 name: self.name(),
                 uuid: Some(self.uuid()),
             })
             .await
-            .map(|r| {
-                let t = r.into_inner();
-                (t.previous_pool.unwrap(), t.current_pool.unwrap())
-            })
+            .map(|r| (r.into_inner()))
     }
 
     pub async fn get_pool(&self) -> Result<Pool, Status> {

io-engine/examples/lvs-eval/main.rs

@@ -33,7 +33,7 @@ pub struct CliArgs {
     pub thin: bool,
     /// MD reservation ratio.
     #[clap(short = 'M')]
-    pub md_resv_ratio: Option<f32>,
+    pub max_expansion: Option<String>,
     /// Use extent table.
     #[clap(short = 'E')]
     pub use_extent_table: Option<bool>,
@@ -133,7 +133,7 @@ async fn create_lvs(args: &CliArgs) -> Lvs {
         uuid: Some(POOL_UUID.to_string()),
         cluster_size: args.cluster_size.map(|sz| sz * 1024 * 1024),
         md_args: Some(PoolMetadataArgs {
-            md_resv_ratio: args.md_resv_ratio,
+            max_expansion: args.max_expansion.clone(),
         }),
         backend: Default::default(),
         enc_key: None,

io-engine/src/bin/io-engine-client/v1/pool_cli.rs

@@ -34,10 +34,10 @@ pub fn subcommands() -> Command {
                 .help("SPDK cluster size"),
         )
         .arg(
-            Arg::new("md-resv-ratio")
-                .long("md-resv-ratio")
+            Arg::new("max-expansion")
+                .long("max-expansion")
                 .required(false)
-                .help("Metadata reservation ratio"),
+                .help("Max expected expansion in factor or absolute size"),
         )
         .arg(
             Arg::new("disk")
@@ -184,8 +184,8 @@ pub fn subcommands() -> Command {
                 .default_value(PoolType::Lvs.as_ref()),
         );
 
-    let grow = Command::new("grow")
-        .about("Grow a storage pool to fill the entire underlying device")
+    let expand = Command::new("expand")
+        .about("Expand a storage pool to span the entire underlying device")
         .arg(
             Arg::new("name")
                 .required(true)
@@ -230,7 +230,7 @@ pub fn subcommands() -> Command {
         .subcommand(import)
         .subcommand(destroy)
         .subcommand(export)
-        .subcommand(grow)
+        .subcommand(expand)
         .subcommand(list)
 }
 
@@ -240,7 +240,7 @@ pub async fn handler(ctx: Context, matches: &ArgMatches) -> crate::Result<()> {
         ("import", args) => import(ctx, args).await,
         ("destroy", args) => destroy(ctx, args).await,
         ("export", args) => export(ctx, args).await,
-        ("grow", args) => grow(ctx, args).await,
+        ("expand", args) => expand(ctx, args).await,
         ("list", args) => list(ctx, args).await,
         (cmd, _) => {
             Err(Status::not_found(format!("command {cmd} does not exist"))).context(GrpcStatus)
@@ -297,8 +297,8 @@ async fn create(mut ctx: Context, matches: &ArgMatches) -> crate::Result<()> {
         None => None,
     };
 
-    let md_resv_ratio = match matches.get_one::<String>("md-resv-ratio") {
-        Some(s) => match s.parse::<f32>() {
+    let max_expansion = match matches.get_one::<String>("max-expansion") {
+        Some(s) => match s.parse::<String>() {
             Ok(v) => Some(v),
             Err(err) => {
                 return Err(Status::invalid_argument(format!(
@@ -336,7 +336,10 @@ async fn create(mut ctx: Context, matches: &ArgMatches) -> crate::Result<()> {
             disks: disks_list,
             pooltype: v1rpc::pool::PoolType::from(pooltype) as i32,
             cluster_size,
-            md_args: Some(v1rpc::pool::PoolMetadataArgs { md_resv_ratio }),
+            md_args: Some(v1rpc::pool::PoolMetadataArgs {
+                md_resv_ratio: None,
+                max_expansion,
+            }),
             encryption: enc_msg,
         })
         .await
@@ -537,7 +540,7 @@ async fn export(mut ctx: Context, matches: &ArgMatches) -> crate::Result<()> {
     Ok(())
 }
 
-async fn grow(mut ctx: Context, matches: &ArgMatches) -> crate::Result<()> {
+async fn expand(mut ctx: Context, matches: &ArgMatches) -> crate::Result<()> {
     let name = matches
         .get_one::<String>("name")
         .ok_or_else(|| ClientError::MissingValue {
@@ -546,24 +549,55 @@ async fn grow(mut ctx: Context, matches: &ArgMatches) -> crate::Result<()> {
         .to_owned();
     let uuid = matches.get_one::<String>("uuid").cloned();
 
-    let response = ctx
+    let pooltype = matches
+        .get_one::<String>("type")
+        .map(|s| PoolType::from_str(s.as_str()))
+        .transpose()
+        .map_err(|e| Status::invalid_argument(e.to_string()))
+        .context(GrpcStatus)?;
+
+    let list_response = ctx
+        .v1
+        .pool
+        .list_pools(v1rpc::pool::ListPoolOptions {
+            name: Some(name.clone()),
+            pooltype: pooltype.map(|pooltype| v1rpc::pool::PoolTypeValue {
+                value: v1rpc::pool::PoolType::from(pooltype) as i32,
+            }),
+            uuid: None,
+        })
+        .await
+        .context(GrpcStatus)?;
+
+    if list_response.get_ref().pools.is_empty() {
+        return Err(ClientError::GrpcStatus {
+            source: Status::not_found(format!("Pool {name} not found")),
+            backtrace: None,
+        });
+    }
+
+    let pool = &list_response.get_ref().pools[0];
+    let previous_capacity = pool.capacity;
+
+    let grow_response = ctx
         .v1
         .pool
-        .grow_pool(v1rpc::pool::GrowPoolRequest {
+        .grow_pool_v2(v1rpc::pool::GrowPoolRequest {
             name: name.clone(),
             uuid,
         })
         .await
         .context(GrpcStatus)?;
 
-    let old_cap = response.get_ref().previous_pool.as_ref().unwrap().capacity;
-    let new_cap = response.get_ref().current_pool.as_ref().unwrap().capacity;
+    let pool = &grow_response.get_ref();
+    let current_capacity = pool.capacity;
 
-    if old_cap == new_cap {
-        println!("Pool capacity did not change: {new_cap} bytes");
-    } else {
-        println!("Pool capacity was {old_cap}, now {new_cap} bytes");
-    }
+    match ctx.output {
+        OutputFormat::Json => {}
+        OutputFormat::Default => {
+            println!("pool expanded from {previous_capacity} to {current_capacity}");
+        }
+    };
 
     Ok(())
 }

io-engine/src/grpc/v0/mayastor_grpc.rs

@@ -250,6 +250,7 @@ impl From<LvsError> for tonic::Status {
             LvsError::SetProperty { .. } => Status::data_loss(e.to_string()),
             LvsError::WipeFailed { source } => source.into(),
             LvsError::ResourceLockFailed { .. } => Status::aborted(e.to_string()),
+            LvsError::MaxExpansionParse { .. } => Status::invalid_argument(e.to_string()),
             _ => Status::internal(e.verbose()),
         }
     }

io-engine/src/grpc/v1/pool.rs

@@ -24,7 +24,7 @@ use std::{
     ops::Deref,
     panic::AssertUnwindSafe,
 };
-use tonic::{Request, Status};
+use tonic::{Code, Request, Status};
 
 pub type PoolCreateEncryptionParams = create_pool_request::Encryption;
 pub type PoolImportEncryptionParams = import_pool_request::Encryption;
@@ -260,7 +260,7 @@ impl TryFrom<CreatePoolRequest> for PoolArgs {
 impl From<PoolMetadataArgs> for pool_backend::PoolMetadataArgs {
     fn from(params: PoolMetadataArgs) -> Self {
         Self {
-            md_resv_ratio: params.md_resv_ratio,
+            max_expansion: params.max_expansion,
         }
     }
 }
@@ -448,6 +448,7 @@ impl From<&dyn PoolOps> for Pool {
             disk_capacity: value.disk_capacity(),
             md_info: value.md_props().map(|md| md.into()),
             encrypted: Some(value.encrypted()),
+            max_expandable_size: value.max_expandable_size(),
         }
     }
 }
@@ -709,39 +710,26 @@ impl PoolRpc for PoolService {
         .await
     }
 
+    async fn grow_pool(&self, _request: Request<GrowPoolRequest>) -> GrpcResult<GrowPoolResponse> {
+        Err(Status::new(
+            Code::Unimplemented,
+            "grow_pool is deprecated. Please use grow_pool_v2",
+        ))
+    }
+
     #[named]
-    async fn grow_pool(&self, request: Request<GrowPoolRequest>) -> GrpcResult<GrowPoolResponse> {
+    async fn grow_pool_v2(&self, request: Request<GrowPoolRequest>) -> GrpcResult<Pool> {
         self.locked(
             GrpcClientContext::new(&request, function_name!()),
             async move {
                 crate::spdk_submit!(async move {
                     info!("{:?}", request.get_ref());
 
                     let pool = GrpcPoolFactory::finder(request.into_inner()).await?;
-
-                    let previous_pool = Pool::from(pool.as_ops());
                     pool.grow().await?;
                     let current_pool = Pool::from(pool.as_ops());
 
-                    if current_pool.capacity == previous_pool.capacity {
-                        info!(
-                            "Grow pool '{p}': capacity did not change: {sz} bytes",
-                            p = current_pool.name,
-                            sz = current_pool.capacity,
-                        );
-                    } else {
-                        info!(
-                            "Grow pool '{p}': pool capacity has changed from {a} to {b} bytes",
-                            p = current_pool.name,
-                            a = previous_pool.capacity,
-                            b = current_pool.capacity
-                        );
-                    }
-
-                    Ok(GrowPoolResponse {
-                        previous_pool: Some(previous_pool),
-                        current_pool: Some(current_pool),
-                    })
+                    Ok(current_pool)
                 })
             },
         )

io-engine/src/lvm/mod.rs

@@ -325,6 +325,10 @@ impl IPoolProps for VolumeGroup {
     fn encrypted(&self) -> bool {
         false
     }
+
+    fn max_expandable_size(&self) -> Option<u64> {
+        None
+    }
 }
 
 /// A factory instance which implements LVM specific `PoolFactory`.

io-engine/src/lvs/lvs_error.rs

@@ -262,6 +262,10 @@ pub enum LvsError {
     ResourceLockFailed {
         msg: String,
     },
+    #[snafu(display("{msg}"))]
+    MaxExpansionParse {
+        msg: String,
+    },
 }
 
 /// Map CoreError to errno code.
@@ -297,6 +301,7 @@ impl ToErrno for LvsError {
             Self::CloneConfigFailed { .. } => Errno::EINVAL,
             Self::WipeFailed { .. } => Errno::EINVAL,
             Self::ResourceLockFailed { .. } => Errno::EBUSY,
+            Self::MaxExpansionParse { .. } => Errno::EINVAL,
         }
     }
 }