From a59e07c1f9ec59d01723ee3d3b694374946b0d20 Mon Sep 17 00:00:00 2001
From: David Gibson <david@gibson.dropbear.id.au>
Date: Tue, 6 Apr 2021 15:02:21 +1000
Subject: [PATCH] agent/define: Refine uevent matching for virtio-scsi devices

Current get_scsi_device_name() uses the legacy uevent matching which
isn't very precise.  This refines it to use a specific matcher
implementation.  While we're at it:

- No longer insist on the SCSI controller being under the PCI root.
  It generally will be, but there's no particular reason to require
  it.

The matcher still has a problem in that it won't work sensibly if
there are multiple SCSI busses in the guest.  Fixing that requires
changes on the runtime side as well, though, so it's beyond scope for
this change.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 src/agent/src/device.rs    | 56 ++++++++++++++++++++++++++++++++++++--
 src/agent/src/linux_abi.rs |  4 ---
 2 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/src/agent/src/device.rs b/src/agent/src/device.rs
index 5034ace688..60fca50df3 100644
--- a/src/agent/src/device.rs
+++ b/src/agent/src/device.rs
@@ -139,14 +139,36 @@ async fn get_device_name(sandbox: &Arc<Mutex<Sandbox>>, dev_addr: &str) -> Resul
     Ok(format!("{}/{}", SYSTEM_DEV_PATH, &uev.devname))
 }
 
+// FIXME: This matcher is only correct if the guest has at most one
+// SCSI host.
+#[derive(Debug)]
+struct ScsiBlockMatcher {
+    search: String,
+}
+
+impl ScsiBlockMatcher {
+    fn new(scsi_addr: &str) -> Result<ScsiBlockMatcher> {
+        let search = format!(r"/0:0:{}/block/", scsi_addr);
+
+        Ok(ScsiBlockMatcher { search })
+    }
+}
+
+impl UeventMatcher for ScsiBlockMatcher {
+    fn is_match(&self, uev: &Uevent) -> bool {
+        uev.subsystem == "block" && uev.devpath.contains(&self.search) && !uev.devname.is_empty()
+    }
+}
+
 pub async fn get_scsi_device_name(
     sandbox: &Arc<Mutex<Sandbox>>,
     scsi_addr: &str,
 ) -> Result<String> {
-    let dev_sub_path = format!("{}{}/{}", SCSI_HOST_CHANNEL, scsi_addr, SCSI_BLOCK_SUFFIX);
+    let matcher = ScsiBlockMatcher::new(scsi_addr)?;
 
     scan_scsi_bus(scsi_addr)?;
-    get_device_name(sandbox, &dev_sub_path).await
+    let uev = wait_for_uevent(sandbox, matcher).await?;
+    Ok(format!("{}/{}", SYSTEM_DEV_PATH, &uev.devname))
 }
 
 #[derive(Debug)]
@@ -870,4 +892,34 @@ mod tests {
         assert!(!matcher_b.is_match(&uev_a));
         assert!(!matcher_a.is_match(&uev_b));
     }
+
+    #[tokio::test]
+    async fn test_scsi_block_matcher() {
+        let root_bus = create_pci_root_bus_path();
+        let devname = "sda";
+
+        let mut uev_a = crate::uevent::Uevent::default();
+        let addr_a = "0:0";
+        uev_a.action = crate::linux_abi::U_EVENT_ACTION_ADD.to_string();
+        uev_a.subsystem = "block".to_string();
+        uev_a.devname = devname.to_string();
+        uev_a.devpath = format!(
+            "{}/0000:00:00.0/virtio0/host0/target0:0:0/0:0:{}/block/sda",
+            root_bus, addr_a
+        );
+        let matcher_a = ScsiBlockMatcher::new(&addr_a).unwrap();
+
+        let mut uev_b = uev_a.clone();
+        let addr_b = "2:0";
+        uev_b.devpath = format!(
+            "{}/0000:00:00.0/virtio0/host0/target0:0:2/0:0:{}/block/sdb",
+            root_bus, addr_b
+        );
+        let matcher_b = ScsiBlockMatcher::new(&addr_b).unwrap();
+
+        assert!(matcher_a.is_match(&uev_a));
+        assert!(matcher_b.is_match(&uev_b));
+        assert!(!matcher_b.is_match(&uev_a));
+        assert!(!matcher_a.is_match(&uev_b));
+    }
 }
diff --git a/src/agent/src/linux_abi.rs b/src/agent/src/linux_abi.rs
index c6e52fa9e9..c95ea09662 100644
--- a/src/agent/src/linux_abi.rs
+++ b/src/agent/src/linux_abi.rs
@@ -78,10 +78,6 @@ pub const SYSFS_MEMORY_BLOCK_SIZE_PATH: &str = "/sys/devices/system/memory/block
 pub const SYSFS_MEMORY_HOTPLUG_PROBE_PATH: &str = "/sys/devices/system/memory/probe";
 pub const SYSFS_MEMORY_ONLINE_PATH: &str = "/sys/devices/system/memory";
 
-// Here in "0:0", the first number is the SCSI host number because
-// only one SCSI controller has been plugged, while the second number
-// is always 0.
-pub const SCSI_HOST_CHANNEL: &str = "0:0:";
 pub const SCSI_BLOCK_SUFFIX: &str = "block";
 pub const SYSFS_SCSI_HOST_PATH: &str = "/sys/class/scsi_host";