udev: refactor ActionHandler to get BD name on remove

On a remove event, the current implementation is unable to
log the name of the BD being removed, because it tries to
get device information by checking /sys/class/block/... and
/var/run/udev/data/... but the device is gone, so that
information is no longer present.  This also results in
some annoying (but harmless) errors in the log:

```
WARNING: failed to read int from file: open /sys/block/sda/queue/rotational: no such file or directory
time="2024-07-16T12:21:55Z" level=info msg="Wake up scanner with remove operation with blockdevice: "
time="2024-07-16T12:21:55Z" level=info msg="scanner waked up, do scan..."
WARNING: failed to read disk partitions: open /sys/block/sda: no such file or directory
time="2024-07-16T12:21:55Z" level=info msg="Waiting new event to trigger..."
```

This commit picks up device information from the udev event
instead, so we're able to avoid those errors and correctly
determine the BD name to log when the device is removed.
So now we get:

```
time="2024-07-16T12:35:14Z" level=info msg="Wake up scanner with remove operation with blockdevice: b3270eab097d4515bac2d595665ac618, device: /dev/sda"
time="2024-07-16T12:35:14Z" level=info msg="scanner waked up, do scan..."
time="2024-07-16T12:35:14Z" level=info msg="Waiting new event to trigger..."
```

Signed-off-by: Tim Serong <tserong@suse.com>

pkg/udev/device.go

@@ -25,6 +25,7 @@ const (
 	UdevPartTableType = "ID_PART_TABLE_TYPE"
 	UdevPartTableUUID = "ID_PART_TABLE_UUID"
 	UdevSerialNumber  = "ID_SERIAL"
+	UdevSerialShort   = "ID_SERIAL_SHORT"
 	UdevType          = "ID_TYPE"
 	UdevVendor        = "ID_VENDOR"
 	UdevWWN           = "ID_WWN"
@@ -36,7 +37,7 @@ func InitUdevDevice(udev map[string]string) Device {
 	return udev
 }
 
-func (device Device) updateDiskFromUdev(disk *block.Disk) {
+func (device Device) UpdateDiskFromUdev(disk *block.Disk) {
 	if len(device[UdevFsUUID]) > 0 {
 		disk.UUID = device[UdevFsUUID]
 	}
@@ -49,7 +50,11 @@ func (device Device) updateDiskFromUdev(disk *block.Disk) {
 	if len(UdevVendor) > 0 {
 		disk.Vendor = device[UdevVendor]
 	}
-	if len(device[UdevSerialNumber]) > 0 {
+	// Match the logic from block.diskSerialNumber() to ensure
+	// we get the correct serial number!
+	if len(device[UdevSerialShort]) > 0 {
+		disk.SerialNumber = device[UdevSerialShort]
+	} else if len(device[UdevSerialNumber]) > 0 {
 		disk.SerialNumber = device[UdevSerialNumber]
 	}
 	if len(device[UdevWWN]) > 0 {

pkg/udev/uevent.go

@@ -5,6 +5,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"os"
+	"strings"
 	"sync"
 
 	"github.com/pilebones/go-udev/netlink"
@@ -103,10 +104,31 @@ func (u *Udev) ActionHandler(uevent netlink.UEvent) {
 	}
 	logrus.Debugf("Prepare to handle event: %s, env: %+v", uevent.Action, uevent.Env)
 
+	devPath := udevDevice.GetDevName()
 	var disk *block.Disk
-	var part *block.Partition
 	var bd *v1beta1.BlockDevice
-	devPath := udevDevice.GetDevName()
+
+	if uevent.Action == netlink.REMOVE {
+		if udevDevice.IsDisk() {
+			// Note: at this point, the device is gone, so we can't use GetDiskByDevPath()
+			// to get any reliable information about the device, and we kinda want its
+			// name for logging purposes.  Happily, we _can_ fill out enough data to
+			// figure out the BD name by creating a minimal block.Disk then calling
+			// UpdateDiskFromUdev() here instead, so the logging works fine.
+			disk = &block.Disk{Name: strings.TrimPrefix(devPath, "/dev/")}
+			udevDevice.UpdateDiskFromUdev(disk)
+			bd = blockdevice.GetDiskBlockDevice(disk, u.nodeName, u.namespace)
+			// just wake up scanner to check if the disk is removed, do no-op internally
+			u.wakeUpScanner(uevent, bd)
+		}
+		return
+	}
+
+	if uevent.Action != netlink.ADD {
+		return
+	}
+
+	var part *block.Partition
 	if udevDevice.IsDisk() {
 		disk = u.scanner.BlockInfo.GetDiskByDevPath(devPath)
 		bd = blockdevice.GetDiskBlockDevice(disk, u.nodeName, u.namespace)
@@ -128,28 +150,21 @@ func (u *Udev) ActionHandler(uevent netlink.UEvent) {
 		return
 	}
 
-	switch uevent.Action {
-	case netlink.ADD:
-		if bd.Name == "" {
-			logrus.Infof("Skip adding non-identifiable block device %s", bd.Spec.DevPath)
-			return
-		}
-		utils.CallerWithCondLock(u.scanner.Cond, func() any {
-			// just wake up scanner to check if the disk is added, do no-op internally
-			logrus.Infof("Wake up scanner with %s operation with blockdevice: %s", netlink.ADD, bd.Name)
-			u.scanner.Cond.Signal()
-			return nil
-		})
-	case netlink.REMOVE:
-		if udevDevice.IsDisk() {
-			// just wake up scanner to check if the disk is removed, do no-op internally
-			utils.CallerWithCondLock(u.scanner.Cond, func() any {
-				logrus.Infof("Wake up scanner with %s operation with blockdevice: %s", netlink.REMOVE, bd.Name)
-				u.scanner.Cond.Signal()
-				return nil
-			})
-		}
+	if bd.Name == "" {
+		logrus.Infof("Skip adding non-identifiable block device %s", bd.Spec.DevPath)
+		return
 	}
+
+	// just wake up scanner to check if the disk is added, do no-op internally
+	u.wakeUpScanner(uevent, bd)
+}
+
+func (u *Udev) wakeUpScanner(uevent netlink.UEvent, bd *v1beta1.BlockDevice) {
+	utils.CallerWithCondLock(u.scanner.Cond, func() any {
+		logrus.Infof("Wake up scanner with %s operation with blockdevice: %s, device: %s", uevent.Action, bd.Name, bd.Spec.DevPath)
+		u.scanner.Cond.Signal()
+		return nil
+	})
 }
 
 // getOptionalMatcher Parse and load config file which contains rules for matching