jaypipes / ghw

Go HardWare discovery/inspection library
Apache License 2.0
1.63k stars 179 forks source link

USB driveType is detected as HDD #122

Open chatenilesh opened 5 years ago

chatenilesh commented 5 years ago

The driveType for USB drive is detected as HDD. The driveType detection logic seems generic.

        } else if strings.HasPrefix(dname, "sd") {
            driveType = DRIVE_TYPE_HDD
            busType = BUS_TYPE_SCSI
            storageController = STORAGE_CONTROLLER_SCSI

What do you think of using ls /dev/disk/by-id/ to detect the correct value. ATA for HDD, USB for USB drive?. Shall I go ahead and code it?

root@nyc:# ls /dev/disk/by-id/ -l
total 0
lrwxrwxrwx 1 root root  9 Feb 13 12:06 ata-VBOX_CD-ROM_VB2-01700376 -> ../../sr0
lrwxrwxrwx 1 root root  9 Feb  7 20:58 ata-VBOX_HARDDISK_VB2cab0c91-5ceaa387 -> ../../sda
lrwxrwxrwx 1 root root 10 Feb  7 20:58 ata-VBOX_HARDDISK_VB2cab0c91-5ceaa387-part1 -> ../../sda1
lrwxrwxrwx 1 root root 10 Feb  7 20:58 ata-VBOX_HARDDISK_VB2cab0c91-5ceaa387-part2 -> ../../sda2
lrwxrwxrwx 1 root root 10 Feb  7 20:58 ata-VBOX_HARDDISK_VB2cab0c91-5ceaa387-part5 -> ../../sda5
lrwxrwxrwx 1 root root  9 Feb 13 12:07 usb-SanDisk_Cruzer_Blade_4C531001490518112283-0:0 -> ../../sdb
jaypipes commented 5 years ago

ATA and USB are the storage bus types, not the drive types, though :)

The listed drive types (so far) are:

If anything, the drive type of the USB flash drive you have above should be SSD, right?

I've considered getting rid of the ghw.Disk.BusType field entirely for this exact reason... it's not particularly useful IMHO and we already have the ghw.Disk.BusPath field if storage bus information is truly wanted by the library user. See the discussion on #117 for the mind-numbing details on this :)

Nilesh, what shows up for the BusPath field in your block devices listed above?

chatenilesh commented 5 years ago

Ok, I got a bit confused after I saw driveType as HDD for both. I think I will use BusPath for now and also look into the code for why it is not detected as DRIVE_TYPE_SSD, sounds good?

Following are the values for BusTypeand BusPath

nyc@nyc:~/Documents$ go run drives.go
Name sda
BusType SCSI
BusPath pci-0000:00:0d.0-ata-1

Name sdb
BusType SCSI
BusPath pci-0000:00:06.0-usb-0:2:1.0-scsi-0:0:0:0
jaypipes commented 5 years ago

Ok, I got a bit confused after I saw driveType as HDD for both. I think I will use BusPath for now and also look into the code for why it is not detected as DRIVE_TYPE_SSD, sounds good?

:+1:

Following are the values for BusTypeand BusPath

nyc@nyc:~/Documents$ go run drives.go
Name sda
BusType SCSI
BusPath pci-0000:00:0d.0-ata-1

Name sdb
BusType SCSI
BusPath pci-0000:00:06.0-usb-0:2:1.0-scsi-0:0:0:0

LOL. Gotta love storage hardware vendors :) That last one lists three separate bus types in the bus path... PCI, USB and SCSI are all bus types. sigh

We'll have to figure out a decent way of determining whether USB drives that show up on a SCSI controller (like the SanDisk_Cruzer_Blade above) are reported as DRIVE_TYPE_SSD. At this point, though, I'm not sure how we're going to consistently detect that... after all, there are USB drives that are spinning rust (I have a couple of those myself)

jpo-joyent commented 5 years ago

What do you think of using ls /dev/disk/by-id/ to detect the correct value. ATA for HDD, USB for USB drive?. Shall I go ahead and code it?

Those filesystem paths are just a side-effect of udev, and you can get the same information (more, actually, in a non-filtered form) from udev directly. In fact, we're already doing exactly that -- see udevInfo() in block_linux.go, and look in your /run/udev/data/ :)

jpo-joyent commented 5 years ago

LOL. Gotta love storage hardware vendors :) That last one lists three separate bus types in the bus path... PCI, USB and SCSI are all bus types. sigh

Hence why it's a bus path rather than just single bus type? Might actually be worth exposing it as a path instead ¯\_(ツ)_/¯

jaypipes commented 5 years ago

LOL. Gotta love storage hardware vendors :) That last one lists three separate bus types in the bus path... PCI, USB and SCSI are all bus types. sigh

Hence why it's a bus path rather than just single bus type? Might actually be worth exposing it as a path instead ¯_(ツ)_/¯

It is already exposed as ghw.Disk.BusPath, @jpo-joyent :)

chatenilesh commented 5 years ago

What do you think of using ls /dev/disk/by-id/ to detect the correct value. ATA for HDD, USB for USB drive?. Shall I go ahead and code it?

Those filesystem paths are just a side-effect of udev, and you can get the same information (more, actually, in a non-filtered form) from udev directly. In fact, we're already doing exactly that -- see udevInfo() in block_linux.go, and look in your /run/udev/data/ :)

Ok, so I had a look at udev and was wondering if it would be a good idea to use the following fields E:ID_BUS, ID_TYPE. Here are the values for above referred devices: HDD E:ID_BUS=ata E:ID_TYPE=disk

USB drive E:ID_BUS=usb E:ID_TYPE=disk E:ID_USB_DRIVER=usb-storage

CD-ROM E:ID_BUS=ata E:ID_TYPE=cd

The above USB drive was detected as HDD because /queue/rotational field is 1 which is because of it being a logical volume/emulated (Correct me here). Not sure what will this field be if I connect SSD and HDD via usb port.

I found a field ID_ATA_ROTATION_RATE_RPM under HDD but not other type of devices. Can we use this field to detect if the device is HDD or SSD, is it reliable?

Would like to go ahead and code if all this sounds good :)

jpo-joyent commented 5 years ago

It may be a good idea to collect a corpus of /sys and udev data from real machines w/ physical descriptions of them so we can actually answer these types of questions. Maybe in a separate repo. I'd gladly contribute a script to collect such info as well as data from a few machines. Thoughts @jaypipes?

I could see such data potentially also being useful for unit tests.

jaypipes commented 5 years ago

It may be a good idea to collect a corpus of /sys and udev data from real machines w/ physical descriptions of them so we can actually answer these types of questions. Maybe in a separate repo. I'd gladly contribute a script to collect such info as well as data from a few machines. Thoughts @jaypipes?

I could see such data potentially also being useful for unit tests.

+100

There is Issue #66 for that. I would definitely welcome those additions, Jean-Philippe. No need for a separate repo. Golang will ignore any directory called "testdata" in the source tree (for build purposes) so I think it would be a good idea to put such flat fixtures in a testdata/ directory.

jaypipes commented 5 years ago

The above USB drive was detected as HDD because /queue/rotational field is 1 which is because of it being a logical volume/emulated (Correct me here). Not sure what will this field be if I connect SSD and HDD via usb port.

:man_shrugging: :) that's what's so fun about storage hardware!

I found a field ID_ATA_ROTATION_RATE_RPM under HDD but not other type of devices. Can we use this field to detect if the device is HDD or SSD, is it reliable?

That's the first I've heard of this udev field. I'm not sure if it's reliable or not. Are you saying that ID_ATA_ROTATION_RATE_RPM does NOT show up for your USB flash storage drive but it DOES show up for the "VBOX_HARDDISK" emulated drive?

Best, -jay

chatenilesh commented 5 years ago

The above USB drive was detected as HDD because /queue/rotational field is 1 which is because of it being a logical volume/emulated (Correct me here). Not sure what will this field be if I connect SSD and HDD via usb port.

🤷‍♂️ :) that's what's so fun about storage hardware!

I found a field ID_ATA_ROTATION_RATE_RPM under HDD but not other type of devices. Can we use this field to detect if the device is HDD or SSD, is it reliable?

That's the first I've heard of this udev field. I'm not sure if it's reliable or not. Are you saying that ID_ATA_ROTATION_RATE_RPM does NOT show up for your USB flash storage drive but it DOES show up for the "VBOX_HARDDISK" emulated drive?

Best, -jay

Yes, I checked on few machines(Physical and VMs) and saw that ID_ATA_ROTATION_RATE_RPM is available only for Hard disks. Maybe we should come to conclusion after verifying on more machines.

jaypipes commented 5 years ago

Hi again @chatenilesh :) Unfortunately, I don't have any machines handy that have disks where ID_ATA_ROTATION_RATE_RPM shows up in the output of udevadm info -q all $DEVICE.

Are you suggesting that we change this code in block_linux.go:

func (ctx *context) diskIsRotational(devName string) bool {
    path := filepath.Join(ctx.pathSysBlock(), devName, "queue", "rotational")
    contents := safeIntFromFile(path)
    return contents == 1
}

to something like this?

func (ctx *context) diskIsRotational(devName string) bool {
    path := filepath.Join(ctx.pathSysBlock(), devName, "queue", "rotational")
    contents := safeIntFromFile(path)
        // If /sys/block/$device/queue/rotational returns 0, it's
        // a sure-fire sign we're an SSD. However, if it returns 1,
        // we may still be an SSD because hardware lies. First check
        // to see if udev returns an ID_ATA_ROTATION_RATE_RPM
        // element. If it doesn't and the bus path indicates this is a USB
        // drive, then return false.
        if contents == 0 {
                return false
        }
        if strings.HasPrefix(ctx.diskBusPath(devName), "usb") {
                info, err := ctx.udevInfo(devName)
                if err != nil {
                        return UNKNOWN
                }
                if _, ok := info["ID_PATH"]; !ok {
                        return false
                }
        }
    return true
}
jpo-joyent commented 5 years ago

Being attached via USB is not a reliable indicator of being rotational or not:

image image image image image image

Storage is hard :/

jaypipes commented 5 years ago

@jpo-joyent I didn't say being attached by USB is a reliable indicator of being rotational. I suggested that for USB drives, we check to see if the ID_ATA_ROTATION_RATE_RPM udev item was found and if so, use that for determining if the drive is HDD or SSD...

chatenilesh commented 5 years ago

@jpo-joyent I didn't say being attached by USB is a reliable indicator of being rotational. I suggested that for USB drives, we check to see if the ID_ATA_ROTATION_RATE_RPM udev item was found and if so, use that for determining if the drive is HDD or SSD...

Even I was thinking of the same and tested a few devices and never came across ID_ATA_ROTATION_RATE_RPM field for USB connection. It seems too complicated to know if the USB attached is SSD/HDD/Flash. May be we have to live with it for now and rely on BusPath or we can set driveType to USBDrive(new) if its has BUS_ID=usb set in udev? USBDrive will act generic value referring to SSD/HDD/Flash,etc connected by USB?

Edit: Something like this https://github.com/jaypipes/ghw/pull/125