fix(nexus): replace NexusChild destroying flag with child state

Replace the destroying flag with a new Destroying ChildState and store
the previous state so that it when handling the remove event it can be
restored if the previous state was Open or Faulted due to OutOfSync to
preserve existing behaviour.

Also prevent a child from being opened if it is already in Destroying
state.

mayastor/src/bdev/nexus/nexus_child.rs

@@ -32,6 +32,8 @@ pub enum ChildError {
     ChildNotClosed {},
     #[snafu(display("Child is faulted, it cannot be reopened"))]
     ChildFaulted {},
+    #[snafu(display("Child is being destroyed"))]
+    ChildBeingDestroyed {},
     #[snafu(display(
         "Child is smaller than parent {} vs {}",
         child_size,
@@ -99,6 +101,8 @@ pub enum ChildState {
     ConfigInvalid,
     /// the child is open for RW
     Open,
+    /// the child is being destroyed
+    Destroying,
     /// the child has been closed by the nexus
     Closed,
     /// the child is faulted
@@ -112,6 +116,7 @@ impl Display for ChildState {
             Self::Init => write!(f, "Init"),
             Self::ConfigInvalid => write!(f, "Config parameters are invalid"),
             Self::Open => write!(f, "Child is open"),
+            Self::Destroying => write!(f, "Child is being destroyed"),
             Self::Closed => write!(f, "Closed"),
         }
     }
@@ -132,9 +137,9 @@ pub struct NexusChild {
     /// current state of the child
     #[serde(skip_serializing)]
     pub state: AtomicCell<ChildState>,
-    /// destruction started, differentiates remove events
+    /// previous state of the child
     #[serde(skip_serializing)]
-    destroying: AtomicCell<bool>,
+    pub prev_state: AtomicCell<ChildState>,
     /// record of most-recent IO errors
     #[serde(skip_serializing)]
     pub(crate) err_store: Option<NexusErrStore>,
@@ -162,15 +167,15 @@ impl Display for NexusChild {
 
 impl NexusChild {
     pub(crate) fn set_state(&self, state: ChildState) {
+        let prev_state = self.state.swap(state);
+        self.prev_state.store(prev_state);
         trace!(
             "{}: child {}: state change from {} to {}",
             self.parent,
             self.name,
-            self.state.load().to_string(),
+            prev_state.to_string(),
             state.to_string(),
         );
-
-        self.state.store(state);
     }
 
     /// Open the child in RW mode and claim the device to be ours. If the child
@@ -182,6 +187,7 @@ impl NexusChild {
     /// A child can only be opened if:
     ///  - it's not faulted
     ///  - it's not already opened
+    ///  - it's not being destroyed
     pub(crate) fn open(
         &mut self,
         parent_size: u64,
@@ -204,6 +210,13 @@ impl NexusChild {
                 info!("called open on an already opened child");
                 return Ok(self.name.clone());
             }
+            ChildState::Destroying => {
+                error!(
+                    "{}: cannot open child {} being destroyed",
+                    self.parent, self.name
+                );
+                return Err(ChildError::ChildBeingDestroyed {});
+            }
             _ => {}
         }
 
@@ -232,7 +245,6 @@ impl NexusChild {
             },
         )?);
 
-        self.destroying.store(false);
         self.desc = Some(desc);
 
         let cfg = Config::get();
@@ -322,11 +334,6 @@ impl NexusChild {
         self.state.load()
     }
 
-    /// returns whether destruction has started
-    pub fn destroying(&self) -> bool {
-        self.destroying.load()
-    }
-
     pub(crate) fn rebuilding(&self) -> bool {
         match RebuildJob::lookup(&self.name) {
             Ok(_) => self.state() == ChildState::Faulted(Reason::OutOfSync),
@@ -362,7 +369,10 @@ impl NexusChild {
 
         // Only wait for bdev removal if the child has been initialised.
         // An uninitialized child won't have an underlying bdev.
-        if self.state.load() != ChildState::Init {
+        // Also check previous state as remove event may not have occurred
+        if self.state.load() != ChildState::Init
+            && self.prev_state.load() != ChildState::Init
+        {
             self.remove_channel.1.next().await;
         }
 
@@ -378,25 +388,35 @@ impl NexusChild {
     pub(crate) fn remove(&mut self) {
         info!("Removing child {}", self.name);
 
-        // Only remove the bdev if the child is being destroyed instead of a
-        // hot remove event e.g. NVMe AEN (asynchronous event notification)
-        let destroying = self.destroying();
-        info!("Destroying child: {}", destroying);
-        if destroying {
+        let mut state = self.state();
+
+        let mut destroying = false;
+        // Only remove the bdev if the child is being destroyed instead of
+        // a hot remove event
+        if state == ChildState::Destroying {
             // The bdev is being removed, so ensure we don't use it again.
             self.bdev = None;
-        }
-
-        let state = self.state();
+            destroying = true;
 
+            state = self.prev_state.load();
+        }
         match state {
             ChildState::Open | Faulted(Reason::OutOfSync) => {
                 // Change the state of the child to ensure it is taken out of
                 // the I/O path when the nexus is reconfigured.
                 self.set_state(ChildState::Closed)
             }
             // leave the state into whatever we found it as
-            _ => {}
+            _ => {
+                if destroying {
+                    // Restore the previous state
+                    info!(
+                        "Restoring previous child state {}",
+                        state.to_string()
+                    );
+                    self.set_state(state);
+                }
+            }
         }
 
         // Remove the child from the I/O path. If we had an IO error the bdev,
@@ -415,8 +435,7 @@ impl NexusChild {
         if destroying {
             // Dropping the last descriptor results in the bdev being removed.
             // This must be performed in this function.
-            let desc = self.desc.take();
-            drop(desc);
+            self.desc.take();
         }
 
         self.remove_complete();
@@ -445,7 +464,7 @@ impl NexusChild {
             parent,
             desc: None,
             state: AtomicCell::new(ChildState::Init),
-            destroying: AtomicCell::new(false),
+            prev_state: AtomicCell::new(ChildState::Init),
             err_store: None,
             remove_channel: mpsc::channel(0),
         }
@@ -455,7 +474,7 @@ impl NexusChild {
     pub(crate) async fn destroy(&self) -> Result<(), NexusBdevError> {
         trace!("destroying child {:?}", self);
         if self.bdev.is_some() {
-            self.destroying.store(true);
+            self.set_state(ChildState::Destroying);
             bdev_destroy(&self.name).await
         } else {
             warn!("Destroy child without bdev");

mayastor/src/grpc/nexus_grpc.rs

@@ -21,6 +21,7 @@ impl From<ChildState> for rpc::ChildState {
             ChildState::Init => rpc::ChildState::ChildDegraded,
             ChildState::ConfigInvalid => rpc::ChildState::ChildFaulted,
             ChildState::Open => rpc::ChildState::ChildOnline,
+            ChildState::Destroying => rpc::ChildState::ChildDegraded,
             ChildState::Closed => rpc::ChildState::ChildDegraded,
             ChildState::Faulted(reason) => match reason {
                 Reason::OutOfSync => rpc::ChildState::ChildDegraded,