[libvirt PATCH 2/4] nodedev: dont rely on ignoring errors on missing properties
Laine Stump
laine at redhat.com
Tue Nov 17 16:04:07 UTC 2020
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 at 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 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")) {
More information about the libvir-list
mailing list