Skip to content

Commit

Permalink
feat(lvs): special error code for out-of-metadata condition
Browse files Browse the repository at this point in the history
* 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 <dmitry.savitskiy@datacore.com>
  • Loading branch information
dsavitskiy committed May 9, 2024
1 parent 435bbc8 commit 349a07c
Show file tree
Hide file tree
Showing 17 changed files with 593 additions and 320 deletions.
2 changes: 1 addition & 1 deletion io-engine/src/bdev/lvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
_ => {
Expand Down
2 changes: 1 addition & 1 deletion io-engine/src/core/share.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down
34 changes: 27 additions & 7 deletions io-engine/src/core/snapshot.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -44,6 +45,7 @@ impl SnapshotParams {
}
}
}

/// Parameters details for the Snapshot Clone.
#[derive(Clone, Debug)]
pub struct CloneParams {
Expand All @@ -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>,
Expand All @@ -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 {
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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.
Expand All @@ -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.
Expand Down
25 changes: 13 additions & 12 deletions io-engine/src/grpc/v0/mayastor_grpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use crate::{
Share,
ShareProps,
SnapshotParams,
ToErrno,
UntypedBdev,
},
grpc::{
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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(
Expand All @@ -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())
}
Expand All @@ -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()),
Expand All @@ -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())
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
});
}
Expand Down Expand Up @@ -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),
})
}
Expand Down
11 changes: 5 additions & 6 deletions io-engine/src/grpc/v1/lvs/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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,
Expand All @@ -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(),
Expand All @@ -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(),
Expand All @@ -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),
});
}
Expand Down
Loading

0 comments on commit 349a07c

Please sign in to comment.