From bfa99c565084d9eb7551950fa77968da39929659 Mon Sep 17 00:00:00 2001 From: Tiago Castro Date: Fri, 25 Aug 2023 13:22:51 +0100 Subject: [PATCH] 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 --- io-engine/src/bdev/nexus/nexus_bdev_error.rs | 3 +++ io-engine/src/grpc/mod.rs | 15 +++++++++++ io-engine/src/grpc/v0/mayastor_grpc.rs | 8 ++++-- io-engine/src/lvs/lvs_error.rs | 8 +++--- io-engine/src/lvs/lvs_store.rs | 28 ++++++++++++++------ 5 files changed, 48 insertions(+), 14 deletions(-) diff --git a/io-engine/src/bdev/nexus/nexus_bdev_error.rs b/io-engine/src/bdev/nexus/nexus_bdev_error.rs index 4e3ee844a..fa0b6032d 100644 --- 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 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 index 9b942f8ff..b5eb65b3d 100644 --- 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 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 index 9cafbd89e..ebaf9eab4 100644 --- a/io-engine/src/grpc/v0/mayastor_grpc.rs +++ b/io-engine/src/grpc/v0/mayastor_grpc.rs @@ -218,8 +218,12 @@ impl From 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 index acb3afc11..384ec5728 100644 --- 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 index 875219be0..e612dd6b1 100644 --- 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), }?;