On 11/17/20 7:56 AM, Daniel P. Berrangé wrote:
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.
Taking you at your word that, e.g. ID_CDROM will *always* be unset, and never set to 0 when the device isn't a CD, Reviewed-by: Laine Stump <laine@redhat.com> (It totally makes sense that this would never happen, and I don't doubt that your statement is correct. I'm just covering myself for the eventuality that someone somewhere did something uh.... "unconventional" :-))
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@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")) {