fix(lvs/error): handle missing errors during import/create

Some error codes not being properly mapped or handled making it impossible to create or import a pool in certain situations. This ensures we handle errors like already exists properly which allows the control-plane to ensure pool creation/import. Signed-off-by: Tiago Castro <[email protected]>
openebs · Aug 25, 2023 · bfa99c5 · bfa99c5
1 parent fca74e0
commit bfa99c5
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 14 deletions.
diff --git a/io-engine/src/bdev/nexus/nexus_bdev_error.rs b/io-engine/src/bdev/nexus/nexus_bdev_error.rs
@@ -294,6 +294,9 @@ impl From<Error> for tonic::Status {
             Error::NameExists {
                 ..
             } => Status::already_exists(e.to_string()),
+            Error::InvalidArguments {
+                ..
+            } => Status::invalid_argument(e.to_string()),
             e => Status::new(Code::Internal, e.verbose()),
         }
     }

diff --git a/io-engine/src/grpc/mod.rs b/io-engine/src/grpc/mod.rs
@@ -6,6 +6,7 @@ use std::{
 };
 
 use futures::channel::oneshot::Receiver;
+use nix::errno::Errno;
 pub use server::MayastorGrpcServer;
 use tonic::{Request, Response, Status};
 
@@ -26,6 +27,20 @@ impl From<BdevError> for tonic::Status {
             BdevError::InvalidUri {
                 ..
             } => Status::invalid_argument(e.to_string()),
+            BdevError::IntParamParseFailed {
+                ..
+            } => Status::invalid_argument(e.to_string()),
+            BdevError::BoolParamParseFailed {
+                ..
+            } => Status::invalid_argument(e.to_string()),
+            BdevError::CreateBdevInvalidParams {
+                source, ..
+            } => match source {
+                Errno::EINVAL => Status::invalid_argument(e.to_string()),
+                Errno::ENOENT => Status::not_found(e.to_string()),
+                Errno::EEXIST => Status::already_exists(e.to_string()),
+                _ => Status::invalid_argument(e.to_string()),
+            },
             e => Status::internal(e.to_string()),
         }
     }

diff --git a/io-engine/src/grpc/v0/mayastor_grpc.rs b/io-engine/src/grpc/v0/mayastor_grpc.rs
@@ -218,8 +218,12 @@ impl From<LvsError> for tonic::Status {
     fn from(e: LvsError) -> Self {
         match e {
             LvsError::Import {
-                ..
-            } => Status::invalid_argument(e.to_string()),
+                source, ..
+            } => match source {
+                Errno::EINVAL => Status::invalid_argument(e.to_string()),
+                Errno::EEXIST => Status::already_exists(e.to_string()),
+                _ => Status::invalid_argument(e.to_string()),
+            },
             LvsError::RepCreate {
                 source, ..
             } => {

diff --git a/io-engine/src/lvs/lvs_error.rs b/io-engine/src/lvs/lvs_error.rs
@@ -11,22 +11,22 @@ use crate::{
 #[derive(Debug, Snafu)]
 #[snafu(visibility(pub(crate)), context(suffix(false)))]
 pub enum Error {
-    #[snafu(display("failed to import pool {}", name))]
+    #[snafu(display("{source}, failed to import pool {name}"))]
     Import {
         source: Errno,
         name: String,
     },
-    #[snafu(display("errno: {} failed to create pool {}", source, name))]
+    #[snafu(display("{source}, failed to create pool {name}"))]
     PoolCreate {
         source: Errno,
         name: String,
     },
-    #[snafu(display("failed to export pool {}", name))]
+    #[snafu(display("{source}, failed to export pool {name}"))]
     Export {
         source: Errno,
         name: String,
     },
-    #[snafu(display("failed to destroy pool {}", name))]
+    #[snafu(display("{source}, failed to destroy pool {name}"))]
     Destroy {
         source: BdevError,
         name: String,

diff --git a/io-engine/src/lvs/lvs_store.rs b/io-engine/src/lvs/lvs_store.rs
@@ -333,10 +333,16 @@ impl Lvs {
                 BdevError::BdevExists {
                     ..
                 } => Ok(parsed.get_name()),
-                _ => Err(Error::InvalidBdev {
-                    source: e,
-                    name: args.disks[0].clone(),
-                }),
+                BdevError::CreateBdevInvalidParams {
+                    source, ..
+                } if source == Errno::EEXIST => Ok(parsed.get_name()),
+                _ => {
+                    tracing::error!("Failed to create pool bdev: {e:?}");
+                    Err(Error::InvalidBdev {
+                        source: e,
+                        name: args.disks[0].clone(),
+                    })
+                }
             },
             Ok(name) => Ok(name),
         }?;
@@ -468,10 +474,16 @@ impl Lvs {
                 BdevError::BdevExists {
                     ..
                 } => Ok(parsed.get_name()),
-                _ => Err(Error::InvalidBdev {
-                    source: e,
-                    name: args.disks[0].clone(),
-                }),
+                BdevError::CreateBdevInvalidParams {
+                    source, ..
+                } if source == Errno::EEXIST => Ok(parsed.get_name()),
+                _ => {
+                    tracing::error!("Failed to create pool bdev: {e:?}");
+                    Err(Error::InvalidBdev {
+                        source: e,
+                        name: args.disks[0].clone(),
+                    })
+                }
             },
             Ok(name) => Ok(name),
         }?;