[libvirt PATCH 2/4] nodedev: dont rely on ignoring errors on missing properties

Daniel P. Berrangé berrange at redhat.com
Tue Nov 17 12:56:06 UTC 2020


The udevProcessStorage method relies on udevGetIntProperty ignoring
errors about non-existant properties and instead setting the value to
zero. In theory when seeing ID_CDROM=1, you might expect that devices
which are not CDs will get ID_CDROM=0, but that's not what happens in
practice. Instead the property simply won't get set at all.

IOW, the code does not need to care about the value of the property,
merely whether it exists or not.

Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
---
 src/node_device/node_device_udev.c | 35 ++++++------------------------
 1 file changed, 7 insertions(+), 28 deletions(-)

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index b1b1886c54..e48e62dab8 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -962,37 +962,16 @@ udevProcessStorage(struct udev_device *device,
 
     if (!storage->drive_type ||
         STREQ(def->caps->data.storage.drive_type, "generic")) {
-        int val = 0;
-        const char *str = NULL;
-
         /* All floppy drives have the ID_DRIVE_FLOPPY prop. This is
          * needed since legacy floppies don't have a drive_type */
-        if (udevGetIntProperty(device, "ID_DRIVE_FLOPPY", &val, 0) < 0)
+        if (udevHasDeviceProperty(device, "ID_DRIVE_FLOPPY"))
+            storage->drive_type = g_strdup("floppy");
+        else if (udevHasDeviceProperty(device, "ID_CDROM"))
+            storage->drive_type = g_strdup("cd");
+        else if (udevHasDeviceProperty(device, "ID_DRIVE_FLASH_SD"))
+            storage->drive_type = g_strdup("sd");
+        else if (udevKludgeStorageType(def) != 0)
             goto cleanup;
-        else if (val == 1)
-            str = "floppy";
-
-        if (!str) {
-            if (udevGetIntProperty(device, "ID_CDROM", &val, 0) < 0)
-                goto cleanup;
-            else if (val == 1)
-                str = "cd";
-        }
-
-        if (!str) {
-            if (udevGetIntProperty(device, "ID_DRIVE_FLASH_SD", &val, 0) < 0)
-                goto cleanup;
-            if (val == 1)
-                str = "sd";
-        }
-
-        if (str) {
-            storage->drive_type = g_strdup(str);
-        } else {
-            /* If udev doesn't have it, perhaps we can guess it. */
-            if (udevKludgeStorageType(def) != 0)
-                goto cleanup;
-        }
     }
 
     if (STREQ(def->caps->data.storage.drive_type, "cd")) {
-- 
2.28.0




More information about the libvir-list mailing list