Various fixes for idempotency, error messages and concurrency

io-engine/src/bdev/nexus/nexus_bdev_error.rs

@@ -131,11 +131,7 @@ pub enum Error {
         name
     ))]
     RemoveLastHealthyChild { child: String, name: String },
-    #[snafu(display(
-        "Cannot remove or offline the last healthy child {} of nexus {}",
-        child,
-        name
-    ))]
+    #[snafu(display("Child {} of nexus {} not found", child, name))]
     ChildNotFound { child: String, name: String },
     #[snafu(display("Child {} of nexus {} is not open", child, name))]
     ChildDeviceNotOpen { child: String, name: String },

io-engine/src/bdev/nexus/nexus_share.rs

@@ -255,7 +255,9 @@ impl<'n> From<&Nexus<'n>> for NexusPtpl {
 impl PtplFileOps for NexusPtpl {
     fn destroy(&self) -> Result<(), std::io::Error> {
         if let Some(path) = self.path() {
-            std::fs::remove_file(path)?;
+            if path.exists() {
+                std::fs::remove_file(path)?;
+            }
         }
         Ok(())
     }

io-engine/src/bdev_api.rs

@@ -80,10 +80,7 @@ pub enum BdevError {
     #[snafu(display("BDEV '{}' could not be found", name))]
     BdevNotFound { name: String },
     // Invalid creation parameters.
-    #[snafu(display(
-        "Failed to create a BDEV '{}': invalid parameters",
-        name
-    ))]
+    #[snafu(display("Failed to create a BDEV '{}'", name))]
     CreateBdevInvalidParams { source: Errno, name: String },
     // Generic creation failure.
     #[snafu(display("Failed to create a BDEV '{}'", name))]

io-engine/src/bin/io-engine-client/v0/nexus_cli.rs

@@ -708,17 +708,18 @@ async fn nexus_publish(
         .get_one::<String>("key")
         .cloned()
         .unwrap_or_default();
-    let protocol = match matches.get_one::<&str>("protocol") {
-        None => v0::ShareProtocolNexus::NexusNbd,
-        Some(&"nvmf") => v0::ShareProtocolNexus::NexusNvmf,
-        Some(_) => {
-            return Err(Status::new(
-                Code::Internal,
-                "Invalid value of share protocol".to_owned(),
-            ))
-            .context(GrpcStatus);
-        }
-    };
+    let protocol =
+        match matches.get_one::<String>("protocol").map(|s| s.as_str()) {
+            None => v0::ShareProtocolNexus::NexusNbd,
+            Some("nvmf") => v0::ShareProtocolNexus::NexusNvmf,
+            Some(_) => {
+                return Err(Status::new(
+                    Code::Internal,
+                    "Invalid value of share protocol".to_owned(),
+                ))
+                .context(GrpcStatus);
+            }
+        };
     let allowed_hosts = matches
         .get_many::<String>("allowed-host")
         .unwrap_or_default()

io-engine/src/bin/io-engine-client/v1/nexus_cli.rs

@@ -579,17 +579,18 @@ async fn nexus_publish(
         .cloned()
         .unwrap_or_default();
 
-    let protocol = match matches.get_one::<&str>("protocol") {
-        None => v1::common::ShareProtocol::Nvmf as i32,
-        Some(&"nvmf") => v1::common::ShareProtocol::Nvmf as i32,
-        Some(_) => {
-            return Err(Status::new(
-                Code::Internal,
-                "Invalid value of share protocol".to_owned(),
-            ))
-            .context(GrpcStatus);
-        }
-    };
+    let protocol =
+        match matches.get_one::<String>("protocol").map(|s| s.as_str()) {
+            None => v1::common::ShareProtocol::Nvmf as i32,
+            Some("nvmf") => v1::common::ShareProtocol::Nvmf as i32,
+            Some(_) => {
+                return Err(Status::new(
+                    Code::Internal,
+                    "Invalid value of share protocol".to_owned(),
+                ))
+                .context(GrpcStatus);
+            }
+        };
     let allowed_hosts = matches
         .get_many::<String>("allowed-host")
         .unwrap_or_default()

io-engine/src/bin/io-engine-client/v1/test_cli.rs

@@ -183,10 +183,14 @@ async fn replica_wipe(
         .map_err(|e| Status::invalid_argument(e.to_string()))
         .context(GrpcStatus)?;
 
-    let chunk_size =
-        parse_size(matches.get_one::<&str>("chunk-size").unwrap_or(&"0"))
-            .map_err(|s| Status::invalid_argument(format!("Bad size '{s}'")))
-            .context(GrpcStatus)?;
+    let chunk_size = parse_size(
+        matches
+            .get_one::<String>("chunk-size")
+            .map(|s| s.as_str())
+            .unwrap_or("0"),
+    )
+    .map_err(|s| Status::invalid_argument(format!("Bad size '{s}'")))
+    .context(GrpcStatus)?;
     let response = ctx
         .v1
         .test

io-engine/src/grpc/mod.rs

@@ -12,7 +12,7 @@ use tonic::{Request, Response, Status};
 
 use crate::{
     bdev_api::BdevError,
-    core::{CoreError, Reactor},
+    core::{CoreError, Reactor, VerboseError},
 };
 
 impl From<BdevError> for tonic::Status {
@@ -33,15 +33,28 @@ impl From<BdevError> for tonic::Status {
             BdevError::BoolParamParseFailed {
                 ..
             } => Status::invalid_argument(e.to_string()),
-            BdevError::CreateBdevInvalidParams {
+            BdevError::UuidParamParseFailed {
+                ..
+            } => Status::invalid_argument(e.to_string()),
+            BdevError::BdevWrongUuid {
+                ..
+            } => Status::invalid_argument(e.to_string()),
+            BdevError::CreateBdevFailed {
+                source, ..
+            }
+            | 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()),
+                Errno::EINVAL => Status::invalid_argument(e.verbose()),
+                Errno::ENOENT => Status::not_found(e.verbose()),
+                Errno::ENODEV => Status::not_found(e.verbose()),
+                Errno::EEXIST => Status::already_exists(e.verbose()),
+                _ => Status::invalid_argument(e.verbose()),
             },
-            e => Status::internal(e.to_string()),
+            BdevError::BdevNotFound {
+                ..
+            } => Status::not_found(e.to_string()),
+            e => Status::internal(e.verbose()),
         }
     }
 }

io-engine/src/lvs/lvs_store.rs

@@ -906,7 +906,9 @@ impl From<&Lvs> for LvsPtpl {
 impl PtplFileOps for LvsPtpl {
     fn destroy(&self) -> Result<(), std::io::Error> {
         if let Some(path) = self.path() {
-            std::fs::remove_dir_all(path)?;
+            if path.exists() {
+                std::fs::remove_dir_all(path)?;
+            }
         }
         Ok(())
     }