From d2caff6c550b53a5a618f35d2f4911e2eeefe68f Mon Sep 17 00:00:00 2001
From: David Gibson <david@gibson.dropbear.id.au>
Date: Thu, 4 Mar 2021 14:38:33 +1100
Subject: [PATCH] agent: Re-organize uevent processing

Uevent::process() is a bit oddly organized.  It treats the onlining of
hotplugged memory as the "default" case, although that's quite specific,
while treating the handling of hotplugged block devices more like a special
case, although that's pretty close to being very general.

Furthermore splitting Uevent::is_block_add_event() from
Uevent::handle_block_add_event() doesn't make a lot of sense, since their
logic is intimately related to each other.

Alter the code to be a bit more sensible: first split on the "action" type
since that's the most fundamental difference, then handle the memory
onlining special case, then the block device add (which will become a lot
more general in future changes).

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 src/agent/src/uevent.rs | 48 ++++++++++++++++++++---------------------
 1 file changed, 24 insertions(+), 24 deletions(-)

diff --git a/src/agent/src/uevent.rs b/src/agent/src/uevent.rs
index 9d7a39fbf3..7f6269ddef 100644
--- a/src/agent/src/uevent.rs
+++ b/src/agent/src/uevent.rs
@@ -51,20 +51,34 @@ impl Uevent {
         event
     }
 
-    // Check whether this is a block device hot-add event.
-    fn is_block_add_event(&self) -> bool {
+    async fn process_add(&self, logger: &Logger, sandbox: &Arc<Mutex<Sandbox>>) {
+        // Special case for memory hot-adds first
+        let online_path = format!("{}/{}/online", SYSFS_DIR, &self.devpath);
+        if online_path.starts_with(SYSFS_MEMORY_ONLINE_PATH) {
+            let _ = online_device(online_path.as_ref()).map_err(|e| {
+                error!(
+                    *logger,
+                    "failed to online device";
+                    "device" => &self.devpath,
+                    "error" => format!("{}", e),
+                )
+            });
+            return;
+        }
+
         let pci_root_bus_path = create_pci_root_bus_path();
-        self.action == U_EVENT_ACTION_ADD
-            && self.subsystem == "block"
+
+        // Check whether this is a block device hot-add event.
+        if !(self.subsystem == "block"
             && {
                 self.devpath.starts_with(pci_root_bus_path.as_str())
                     || self.devpath.starts_with(ACPI_DEV_PATH) // NVDIMM/PMEM devices
             }
-            && !self.devname.is_empty()
-    }
+            && !self.devname.is_empty())
+        {
+            return;
+        }
 
-    async fn handle_block_add_event(&self, sandbox: &Arc<Mutex<Sandbox>>) {
-        let pci_root_bus_path = create_pci_root_bus_path();
         let mut sb = sandbox.lock().await;
 
         // Record the event by sysfs path
@@ -95,22 +109,8 @@ impl Uevent {
     }
 
     async fn process(&self, logger: &Logger, sandbox: &Arc<Mutex<Sandbox>>) {
-        if self.is_block_add_event() {
-            return self.handle_block_add_event(sandbox).await;
-        } else if self.action == U_EVENT_ACTION_ADD {
-            let online_path = format!("{}/{}/online", SYSFS_DIR, &self.devpath);
-            // It's a memory hot-add event.
-            if online_path.starts_with(SYSFS_MEMORY_ONLINE_PATH) {
-                let _ = online_device(online_path.as_ref()).map_err(|e| {
-                    error!(
-                        *logger,
-                        "failed to online device";
-                        "device" => &self.devpath,
-                        "error" => format!("{}", e),
-                    )
-                });
-                return;
-            }
+        if self.action == U_EVENT_ACTION_ADD {
+            return self.process_add(logger, sandbox).await;
         }
         debug!(*logger, "ignoring event"; "uevent" => format!("{:?}", self));
     }