[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [PATCH 4/5] node_device: detect DASD devices

On 9/8/20 5:50 PM, Erik Skultety wrote:
On Mon, Aug 24, 2020 at 01:59:14PM +0200, Bjoern Walk wrote:
From: Boris Fiuczynski <fiuczy linux ibm com>

Make Direct Access Storage Devices (DASDs) available in the node_device driver.

Reviewed-by: Bjoern Walk <bwalk linux ibm com>
Signed-off-by: Boris Fiuczynski <fiuczy linux ibm com>
  src/node_device/node_device_udev.c | 24 ++++++++++++++++++++++++
  1 file changed, 24 insertions(+)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 1c4df709..5f9d67cc 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -871,6 +871,19 @@ udevProcessSD(struct udev_device *device,

+static int
+udevProcessDasd(struct udev_device *device,
+                virNodeDeviceDefPtr def)


Changed it

+    virNodeDevCapStoragePtr storage = &def->caps->data.storage;
+    if (udevGetStringSysfsAttr(device, "device/uid", &storage->serial) < 0)
+        return -1;
+    return udevProcessDisk(device, def);
  /* This function exists to deal with the case in which a driver does
   * not provide a device type in the usual place, but udev told us it's
   * a storage device, and we can make a good guess at what kind of
@@ -891,6 +904,15 @@ udevKludgeStorageType(virNodeDeviceDefPtr def)
          return 0;
+    /* dasd disk */
+    if (STRPREFIX(def->caps->data.storage.block, "/dev/dasd")) {
+        def->caps->data.storage.drive_type = g_strdup("dasd");
+        VIR_DEBUG("Found storage type '%s' for device "
+                  "with sysfs path '%s'",
+                  def->caps->data.storage.drive_type,
+                  def->sysfs_path);

I understand why we would need it for /dev/vdX, but can udev not know the
drive_type from kernel? IOW Do we really need ^this hunk?

For DASDs there are currently no identifies in udev besides ID_PATH.
ID_TYPE=disk does not exist. That's why the DASDs fall through the udevProcessStorage detection logic. Without this hunk the dasd devices are being detected as storage devices but than end up as "Unsupported storage type".
The short answer is yes. :-)

+        return 0;
+    }
      VIR_DEBUG("Could not determine storage type "
                "for device with sysfs path '%s'", def->sysfs_path);
      return -1;
@@ -978,6 +1000,8 @@ udevProcessStorage(struct udev_device *device,
          ret = udevProcessFloppy(device, def);
      } else if (STREQ(def->caps->data.storage.drive_type, "sd")) {
          ret = udevProcessSD(device, def);
+    } else if (STREQ(def->caps->data.storage.drive_type, "dasd")) {
+        ret = udevProcessDasd(device, def);
      } else {
          VIR_DEBUG("Unsupported storage type '%s'",

The rest looks good:
Reviewed-by: Erik Skultety <eskultet redhat com>


Mit freundlichen Grüßen/Kind regards
   Boris Fiuczynski

IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Gregor Pillen
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]