feat(lvs): special error code for out-of-metadata condition

* Previously, when a blob store ran out of free metadata page during
volume creation, an ambigious ENOMEM error was returned. Now, in order
to allow to have meaningful client-faced error message and codes, EMFILE is
returned instead.
* A test for a proper error when exceeding pool metadata capacity was added.
* LVS errors refactored to eliminate the use of low-level Errno in
high-level LVS code.
* Low-level Errno codes from SPDK are now mapped to high-level error enum.
* Several bugs in Errno conversion fixed in LVS code (negative vs positive
errno in SPDK function return value).

Signed-off-by: Dmitry Savitskiy <[email protected]>

io-engine/src/bdev/lvs.rs

@@ -220,7 +220,7 @@ impl Lvs {
         match &self.mode {
             LvsMode::Create => {
                 match crate::lvs::Lvs::import_from_args(args.clone()).await {
-                    Err(crate::lvs::Error::Import {
+                    Err(crate::lvs::LvsError::Import {
                         ..
                     }) => crate::lvs::Lvs::create_or_import(args).await,
                     _ => {

io-engine/src/core/share.rs

@@ -2,7 +2,7 @@ use async_trait::async_trait;
 use pin_utils::core_reexport::fmt::Formatter;
 use std::{convert::TryFrom, fmt::Display, pin::Pin};
 
-use crate::lvs::Error as LvsError;
+use crate::lvs::LvsError;
 
 /// Indicates what protocol the bdev is shared as.
 #[derive(Debug, Default, PartialOrd, Eq, PartialEq, Copy, Clone)]

io-engine/src/core/snapshot.rs

@@ -1,6 +1,7 @@
 use crate::{lvs::Lvol, subsys::NvmfReq};
 use async_trait::async_trait;
 use futures::channel::oneshot;
+use nix::errno::Errno;
 use serde::{Deserialize, Serialize};
 use spdk_rs::libspdk::{spdk_lvol, spdk_xattr_descriptor};
 use std::{
@@ -44,6 +45,7 @@ impl SnapshotParams {
         }
     }
 }
+
 /// Parameters details for the Snapshot Clone.
 #[derive(Clone, Debug)]
 pub struct CloneParams {
@@ -56,6 +58,7 @@ pub struct CloneParams {
     /// Timestamp when the clone is created.
     pub clone_create_time: Option<String>,
 }
+
 impl CloneParams {
     pub fn new(
         clone_name: Option<String>,
@@ -70,39 +73,48 @@ impl CloneParams {
             clone_create_time,
         }
     }
+
     /// Get clone name.
     pub fn clone_name(&self) -> Option<String> {
         self.clone_name.clone()
     }
+
     /// Set clone name.
     pub fn set_clone_name(&mut self, clone_name: String) {
         self.clone_name = Some(clone_name);
     }
+
     /// Get clone uuid.
     pub fn clone_uuid(&self) -> Option<String> {
         self.clone_uuid.clone()
     }
+
     /// Set clone uuid.
     pub fn set_clone_uuid(&mut self, clone_uuid: String) {
         self.clone_uuid = Some(clone_uuid);
     }
+
     /// Get source uuid from which clone is created.
     pub fn source_uuid(&self) -> Option<String> {
         self.source_uuid.clone()
     }
+
     /// Set source uuid.
     pub fn set_source_uuid(&mut self, uuid: String) {
         self.source_uuid = Some(uuid);
     }
+
     /// Get clone creation time.
     pub fn clone_create_time(&self) -> Option<String> {
         self.clone_create_time.clone()
     }
+
     /// Set clone create time.
     pub fn set_clone_create_time(&mut self, time: String) {
         self.clone_create_time = Some(time);
     }
 }
+
 /// Snapshot Descriptor to respond back as part of listsnapshot.
 #[derive(Clone, Debug)]
 pub struct VolumeSnapshotDescriptor {
@@ -115,6 +127,7 @@ pub struct VolumeSnapshotDescriptor {
     // set to false, if any of the snapshotdescriptor is not filled properly
     pub valid_snapshot: bool,
 }
+
 impl VolumeSnapshotDescriptor {
     pub fn new(
         snapshot_lvol: Lvol,
@@ -190,13 +203,15 @@ impl SnapshotXattrs {
         }
     }
 }
+
 /// Clone attributes used to store its properties.
 #[derive(Debug, EnumCountMacro, EnumIter)]
 pub enum CloneXattrs {
     CloneUuid,
     SourceUuid,
     CloneCreateTime,
 }
+
 impl CloneXattrs {
     pub fn name(&self) -> &'static str {
         match *self {
@@ -206,6 +221,10 @@ impl CloneXattrs {
         }
     }
 }
+
+/// Result for low-level Lvol calls.
+pub type LvolResult = Result<*mut spdk_lvol, Errno>;
+
 ///  Traits gives the common snapshot/clone interface for Local/Remote Lvol.
 #[async_trait(?Send)]
 pub trait SnapshotOps {
@@ -278,30 +297,31 @@ pub trait SnapshotOps {
         params: SnapshotParams,
         cstrs: &mut Vec<CString>,
     ) -> Result<(), Self::Error>;
+
     /// create replica snapshot inner function to call spdk snapshot create
     /// function.
     unsafe fn create_snapshot_inner(
         &self,
         snap_param: &SnapshotParams,
         done_cb: unsafe extern "C" fn(*mut c_void, *mut spdk_lvol, i32),
-        done_cb_arg: *mut ::std::os::raw::c_void,
+        done_cb_arg: *mut c_void,
     ) -> Result<(), Self::Error>;
 
     /// Supporting function for creating local snapshot.
     async fn do_create_snapshot(
         &self,
         snap_param: SnapshotParams,
         done_cb: unsafe extern "C" fn(*mut c_void, *mut spdk_lvol, i32),
-        done_cb_arg: *mut ::std::os::raw::c_void,
-        receiver: oneshot::Receiver<(i32, *mut spdk_lvol)>,
+        done_cb_arg: *mut c_void,
+        receiver: oneshot::Receiver<LvolResult>,
     ) -> Result<Self::Lvol, Self::Error>;
 
     /// Supporting function for creating remote snapshot.
     async fn do_create_snapshot_remote(
         &self,
         snap_param: SnapshotParams,
         done_cb: unsafe extern "C" fn(*mut c_void, *mut spdk_lvol, i32),
-        done_cb_arg: *mut ::std::os::raw::c_void,
+        done_cb_arg: *mut c_void,
     ) -> Result<(), Self::Error>;
 
     /// Prepare clone xattrs.
@@ -317,16 +337,16 @@ pub trait SnapshotOps {
         &self,
         clone_param: &CloneParams,
         done_cb: unsafe extern "C" fn(*mut c_void, *mut spdk_lvol, i32),
-        done_cb_arg: *mut ::std::os::raw::c_void,
+        done_cb_arg: *mut c_void,
     ) -> Result<(), Self::Error>;
 
     /// Supporting function for creating clone.
     async fn do_create_clone(
         &self,
         clone_param: CloneParams,
         done_cb: unsafe extern "C" fn(*mut c_void, *mut spdk_lvol, i32),
-        done_cb_arg: *mut ::std::os::raw::c_void,
-        receiver: oneshot::Receiver<(i32, *mut spdk_lvol)>,
+        done_cb_arg: *mut c_void,
+        receiver: oneshot::Receiver<LvolResult>,
     ) -> Result<Self::Lvol, Self::Error>;
 
     /// Common API to set SnapshotDescriptor for ListReplicaSnapshot.

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

@@ -30,6 +30,7 @@ use crate::{
         Share,
         ShareProps,
         SnapshotParams,
+        ToErrno,
         UntypedBdev,
     },
     grpc::{
@@ -50,7 +51,7 @@ use crate::{
         Serializer,
     },
     host::{blk_device, resource},
-    lvs::{lvs_lvol::LvsLvol, Error as LvsError, Lvol, Lvs},
+    lvs::{lvs_lvol::LvsLvol, BsError, Lvol, Lvs, LvsError},
     pool_backend::PoolArgs,
     rebuild::{RebuildState, RebuildStats},
     subsys::PoolConfig,
@@ -202,7 +203,7 @@ impl TryFrom<CreatePoolRequest> for PoolArgs {
     fn try_from(args: CreatePoolRequest) -> Result<Self, Self::Error> {
         match args.disks.len() {
             0 => Err(LvsError::Invalid {
-                source: Errno::EINVAL,
+                source: BsError::InvalidArgument {},
                 msg: "invalid argument, missing devices".to_string(),
             }),
             _ => Ok(Self {
@@ -219,23 +220,23 @@ impl From<LvsError> for tonic::Status {
         match e {
             LvsError::Import {
                 source, ..
-            } => match source {
+            } => match source.to_errno() {
                 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, ..
             } => {
-                if source == Errno::ENOSPC {
+                if source.to_errno() == Errno::ENOSPC {
                     Status::resource_exhausted(e.to_string())
                 } else {
                     Status::invalid_argument(e.to_string())
                 }
             }
             LvsError::RepDestroy {
                 source, ..
-            } => match source {
+            } => match source.to_errno() {
                 Errno::ENOENT => {
                     let mut status = Status::not_found(e.to_string());
                     status.metadata_mut().insert(
@@ -250,7 +251,7 @@ impl From<LvsError> for tonic::Status {
             },
             LvsError::RepResize {
                 source, ..
-            } => match source {
+            } => match source.to_errno() {
                 Errno::ENOSPC | Errno::ENOMEM => {
                     Status::resource_exhausted(e.to_string())
                 }
@@ -271,7 +272,7 @@ impl From<LvsError> for tonic::Status {
             } => source.into(),
             LvsError::Invalid {
                 source, ..
-            } => match source {
+            } => match source.to_errno() {
                 Errno::EINVAL => Status::invalid_argument(e.to_string()),
                 Errno::ENOMEDIUM => Status::failed_precondition(e.to_string()),
                 Errno::ENOENT => Status::not_found(e.to_string()),
@@ -284,9 +285,9 @@ impl From<LvsError> for tonic::Status {
             LvsError::PoolCreate {
                 source, ..
             } => {
-                if source == Errno::EEXIST {
+                if source.to_errno() == Errno::EEXIST {
                     Status::already_exists(e.to_string())
-                } else if source == Errno::EINVAL {
+                } else if source.to_errno() == Errno::EINVAL {
                     Status::invalid_argument(e.to_string())
                 } else {
                     Status::internal(e.to_string())
@@ -599,7 +600,7 @@ impl mayastor_server::Mayastor for MayastorSvc {
                             Err(LvsError::PoolCreate {
                                 source,
                                 name,
-                            }) if source == Errno::EEXIST => {
+                            }) if source.to_errno() == Errno::EEXIST => {
                                 info!(
                                     "returning already created pool {}",
                                     name,
@@ -693,7 +694,7 @@ impl mayastor_server::Mayastor for MayastorSvc {
 
                     if Lvs::lookup(&args.pool).is_none() {
                         return Err(LvsError::Invalid {
-                            source: Errno::ENOSYS,
+                            source: BsError::from_errno(Errno::ENOSYS),
                             msg: format!("Pool {} not found", args.pool),
                         });
                     }
@@ -787,7 +788,7 @@ impl mayastor_server::Mayastor for MayastorSvc {
                         Some(lvs) => lvs,
                         None => {
                             return Err(LvsError::Invalid {
-                                source: Errno::ENOSYS,
+                                source: BsError::from_errno(Errno::ENOSYS),
                                 msg: format!("Pool {} not found", args.pool),
                             })
                         }

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

@@ -7,11 +7,10 @@ use crate::{
         v1::pool::PoolProbe,
         GrpcResult,
     },
-    lvs::{Error as LvsError, Lvs},
+    lvs::{BsError, Lvs, LvsError},
     pool_backend::PoolArgs,
 };
 use io_engine_api::v1::pool::*;
-use nix::errno::Errno;
 use std::{convert::TryFrom, fmt::Debug};
 use tonic::{Request, Response, Status};
 
@@ -119,7 +118,7 @@ impl PoolRpc for PoolService {
         let rx = rpc_submit::<_, _, LvsError>(async move {
             let Some(pool) = Lvs::lookup(&args.name) else {
                 return Err(LvsError::PoolNotFound {
-                    source: Errno::ENOMEDIUM,
+                    source: BsError::LvsNotFound {},
                     msg: format!(
                         "Destroy failed as pool {} was not found",
                         args.name,
@@ -128,7 +127,7 @@ impl PoolRpc for PoolService {
             };
             if args.uuid.is_some() && args.uuid != Some(pool.uuid()) {
                 return Err(LvsError::Invalid {
-                    source: Errno::EINVAL,
+                    source: BsError::InvalidArgument {},
                     msg: format!(
                         "invalid uuid {}, found pool with uuid {}",
                         args.uuid.unwrap(),
@@ -153,7 +152,7 @@ impl PoolRpc for PoolService {
             if let Some(pool) = Lvs::lookup(&args.name) {
                 if args.uuid.is_some() && args.uuid != Some(pool.uuid()) {
                     return Err(LvsError::Invalid {
-                        source: Errno::EINVAL,
+                        source: BsError::InvalidArgument {},
                         msg: format!(
                             "invalid uuid {}, found pool with uuid {}",
                             args.uuid.unwrap(),
@@ -164,7 +163,7 @@ impl PoolRpc for PoolService {
                 pool.export().await?;
             } else {
                 return Err(LvsError::Invalid {
-                    source: Errno::EINVAL,
+                    source: BsError::InvalidArgument {},
                     msg: format!("pool {} not found", args.name),
                 });
             }