Merge #1897

1897: do bdev rescan before calling lvs grow function r=abhilashshetty04 a=abhilashshetty04

This PR does `bdev_aio_rescan` before calling grow function.

Also some changes in error handling which will help in control plane logic.

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

Author: mayastor-bors

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

@@ -251,6 +251,26 @@ impl From<LvsError> for tonic::Status {
             LvsError::WipeFailed { source } => source.into(),
             LvsError::ResourceLockFailed { .. } => Status::aborted(e.to_string()),
             LvsError::MaxExpansionParse { .. } => Status::invalid_argument(e.to_string()),
+            LvsError::BdevRescanFailed { .. } => {
+                let mut status = Status::failed_precondition(e.to_string());
+                status.metadata_mut().insert(
+                    "bdev_rescan_failed",
+                    tonic::metadata::MetadataValue::from(0),
+                );
+                status
+            }
+            LvsError::BdevNotExtended { .. } => {
+                let mut status = Status::failed_precondition(e.to_string());
+                status
+                    .metadata_mut()
+                    .insert("bdev_not_extended", tonic::metadata::MetadataValue::from(0));
+                status
+            }
+            LvsError::Grow { source, .. } => match source.to_errno() {
+                Errno::ENOMEM => Status::resource_exhausted(e.to_string()),
+                Errno::ENOSPC => Status::out_of_range(e.to_string()),
+                _ => Status::internal(e.to_string()),
+            },
             _ => Status::internal(e.verbose()),
         }
     }

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

@@ -724,11 +724,9 @@ impl PoolRpc for PoolService {
             async move {
                 crate::spdk_submit!(async move {
                     info!("{:?}", request.get_ref());
-
                     let pool = GrpcPoolFactory::finder(request.into_inner()).await?;
                     pool.grow().await?;
                     let current_pool = Pool::from(pool.as_ops());
-
                     Ok(current_pool)
                 })
             },

io-engine/src/lvs/lvs_error.rs

@@ -266,6 +266,15 @@ pub enum LvsError {
     MaxExpansionParse {
         msg: String,
     },
+    #[snafu(display("{source}, Failed to rescan bdev {name}"))]
+    BdevRescanFailed {
+        source: BsError,
+        name: String,
+    },
+    #[snafu(display("Failed to extend pool bdev: {name}"))]
+    BdevNotExtended {
+        name: String,
+    },
 }
 
 /// Map CoreError to errno code.
@@ -302,6 +311,8 @@ impl ToErrno for LvsError {
             Self::WipeFailed { .. } => Errno::EINVAL,
             Self::ResourceLockFailed { .. } => Errno::EBUSY,
             Self::MaxExpansionParse { .. } => Errno::EINVAL,
+            Self::BdevRescanFailed { source, .. } => source.to_errno(),
+            Self::BdevNotExtended { .. } => Errno::EOPNOTSUPP,
         }
     }
 }

io-engine/src/lvs/lvs_store.rs

@@ -8,7 +8,7 @@ use nix::errno::Errno;
 use pin_utils::core_reexport::fmt::Formatter;
 
 use spdk_rs::libspdk::{
-    spdk_bdev_update_bs_blockcnt, spdk_blob_store, spdk_bs_free_cluster_count,
+    bdev_aio_rescan, spdk_bdev_update_bs_blockcnt, spdk_blob_store, spdk_bs_free_cluster_count,
     spdk_bs_get_cluster_size, spdk_bs_get_md_len, spdk_bs_get_page_size, spdk_bs_get_used_md,
     spdk_bs_total_data_cluster_count, spdk_lvol, spdk_lvol_opts, spdk_lvol_opts_init,
     spdk_lvol_store, spdk_lvs_grow_live, vbdev_get_lvol_store_by_name,
@@ -874,11 +874,40 @@ impl Lvs {
         Ok(())
     }
 
-    /// Grows the online (live) pool.
+    /// Rescans the bdev and triggers live LVS grow i.e. without closing the blobs and unloading the blobstore.
     #[tracing::instrument(level = "debug", err)]
     pub async fn grow(&self) -> Result<(), LvsError> {
         info!("{self:?}: growing lvs...");
 
+        let cname = self.base_bdev().name().into_cstring();
+        let uri_str = &self.base_bdev().bdev_uri_str().unwrap_or_default();
+        let url = Url::parse(uri_str).map_err(|source| LvsError::InvalidBdev {
+            source: BdevError::UriParseFailed {
+                source,
+                uri: uri_str.to_string(),
+            },
+            name: self.name().to_string(),
+        })?;
+        if url.scheme() != "malloc" {
+            info!(
+                "Attempting to rescan bdev {:?} part of lvs {:?}, uri {:?}",
+                self.base_bdev().name(),
+                self.name(),
+                self.base_bdev().bdev_uri_str().unwrap_or_default()
+            );
+
+            let errno = unsafe { bdev_aio_rescan(cname.as_ptr() as *mut std::os::raw::c_char) };
+
+            if errno != 0 {
+                return Err(LvsError::BdevRescanFailed {
+                    source: BsError::from_i32(errno),
+                    name: self.base_bdev().name().to_string(),
+                });
+            }
+        }
+
+        let capacity_before_grow = self.capacity();
+
         let (s, r) = pair::<i32>();
 
         unsafe {
@@ -898,6 +927,12 @@ impl Lvs {
                 name: self.name().to_string(),
             })?;
 
+        if self.capacity() == capacity_before_grow {
+            return Err(LvsError::BdevNotExtended {
+                name: self.base_bdev().name().to_string(),
+            });
+        }
+
         info!("{self:?}: lvs has been grown successfully");
 
         Ok(())

io-engine/tests/lvs_grow.rs

@@ -82,6 +82,7 @@ impl From<Pool> for TestPoolStats {
 /// Grow test interface.
 #[async_trait(?Send)]
 trait GrowTest {
+    fn is_malloc(&self) -> bool;
     async fn create_pool(&mut self) -> TestPoolStats;
     async fn pool_stats(&self) -> TestPoolStats;
     async fn grow_pool(&mut self) -> (TestPoolStats, TestPoolStats);
@@ -107,19 +108,22 @@ async fn test_grow(create: impl Future<Output = Box<dyn GrowTest>>) {
     // Pool capacity must not change, disk capacity must reflect disk size
     // change.
     let after_dev_grow = gt.pool_stats().await;
+    if gt.is_malloc() {
+        assert_eq!(after_dev_grow.disk_capacity, new_dev_cap);
+    } else {
+        assert!(after_dev_grow.disk_capacity < new_dev_cap)
+    }
     assert_eq!(after_dev_grow.capacity, initial.capacity);
-    assert_eq!(after_dev_grow.disk_capacity, new_dev_cap);
 
     // Grow the pool.
     let (before_pool_grow, after_pool_grow) = gt.grow_pool().await;
     assert_eq!(before_pool_grow.capacity, initial.capacity);
-    assert_eq!(before_pool_grow.disk_capacity, new_dev_cap);
 
     // Pool must have grown.
     assert!(after_pool_grow.capacity > before_pool_grow.capacity);
 
     // New pool capacity must be close to the disk capacity.
-    assert!(after_pool_grow.capacity <= after_pool_grow.disk_capacity);
+    assert!(after_pool_grow.capacity < after_pool_grow.disk_capacity);
     assert!(after_pool_grow.capacity_approx_matches());
 }
 
@@ -194,6 +198,10 @@ async fn lvs_grow_ms_malloc() {
             .await;
             self.device_size().await
         }
+
+        fn is_malloc(&self) -> bool {
+            true
+        }
     }
 
     test_grow(async { Box::new(GrowTestMsMalloc {}) as Box<dyn GrowTest> }).await;
@@ -268,6 +276,10 @@ async fn lvs_grow_api_malloc() {
             let bdev = create_bdev(self.ms.clone(), BDEV_URI_RESIZE).await.unwrap();
             bdev.num_blocks * bdev.blk_size as u64
         }
+
+        fn is_malloc(&self) -> bool {
+            true
+        }
     }
 
     test_grow(async { Box::new(GrowTestApiMalloc::new().await) as Box<dyn GrowTest> }).await;
@@ -279,7 +291,6 @@ async fn lvs_grow_api_aio() {
     const DISK_NAME: &str = "/tmp/disk1.img";
     const BDEV_NAME: &str = "/host/tmp/disk1.img";
     const BDEV_URI: &str = "aio:///host/tmp/disk1.img?blk_size=512";
-    const BDEV_URI_RESCAN: &str = "aio:///host/tmp/disk1.img?blk_size=512&rescan";
     const POOL_NAME: &str = "pool0";
     const POOL_UUID: &str = "40baf8b5-6256-4f29-b073-61ebf67d9b91";
 
@@ -347,10 +358,11 @@ async fn lvs_grow_api_aio() {
         async fn grow_device(&mut self) -> u64 {
             // Resize bdev's backing file.
             common::truncate_file(DISK_NAME, 128 * 1024);
+            self.device_size().await + (128 * 1024)
+        }
 
-            // Rescan AIO bdev (re-read its size from the backing media).
-            let bdev = create_bdev(self.ms.clone(), BDEV_URI_RESCAN).await.unwrap();
-            bdev.num_blocks * bdev.blk_size as u64
+        fn is_malloc(&self) -> bool {
+            false
         }
     }