GNOME Bugzilla – Bug 751251
Show serial number in device information
Last modified: 2015-08-03 17:31:37 UTC
It would be useful to display the device serial number in the device information. Without this I have to run a separate command to ensure I have selected the right device.
Hi Adrian, This seems like a perfectly reasonable request. I'll take a look at implementing it. Mike
Hi Curtis, This isn't trivial because libparted doesn't report the serial number GParted calls: Ped_Device * ped_device_get(const char * path) to get information from libparted about the device but the Ped_Device structure only contains a model string, but not a serial number. https://www.gnu.org/software/parted/api/struct__PedDevice.html Ways that I know of to display the model and serial number of a hard drive: hdparm -I /dev/sda | egrep 'Model|Serial' smartctl -i /dev/sda | egrep 'Model|Serial' lshw -class disk | egrep 'product|serial|logical name' lsblk --nodeps -o name,model,serial (Serial column added in util-linux 2.24-rc1. Util-linux 2.24 released 2013-10-21.) There is no kernel /proc or /sys file which contains the serial number. # strace hdparm -I /dev/sda ... open("/dev/sda", O_RDONLY|O_NONBLOCK) = 3 ioctl(3, SG_IO, {'S', SG_DXFER_FROM_DEV, cmd[16]=[85, 08, 0e, 00, 00, 00, 01, 00, 00, 00, 00, 00, 00, 40, ec, 00], mx_sb_len=32, iovec_count=0, dxfer_len=512, timeout=15000, flags=0, data[512]=["zB\377?7\310\20\0\0\0\0\0?\0\0\0\0\0\0\0 W -DCWYA"...], status=00, masked_status=00, sb[0]=[], host_status=0, driver_status=0, resid=0, duration=11, info=0}) = 0 close(3) = 0 ... /dev/sda: ATA device, with non-removable media Model Number: WDC WD2500AAKX-083CA0 Serial Number: WD-WCAYW0568459 Firmware Revision: 15.01H15 Transport: Serial, SATA 1.0a, SATA ... So either GParted will have to do an ioctl() and interpret the data block or call one of the above commands and parse the output to get the serial number. As GParted already calls external commands and parses output for many other items of data, this is the direction I will take. Mike
Hi Curtis, Can you run "hdparm -I /dev/RAIDARRAY" on your machine with a BIOS RAID and paste the first 10 lines or so. Also do you have any suggestions for what to display for the serial number when it isn't available. Cases I can think of: 1) Device doesn't have a serial number E.g. Linux software RAID arrays (mdadm) and Linux DM RAID (dmraid) 2) Error querying serial number 3) hdparm command is missing so can't query serial number 4) Other unknown device type which doesn't have serial numbers I'm thinking of "none" if the device is known not to have serial numbers and blank or "unknown" otherwise. Example error output from Linux Software RAID Array: # hdparm -I /dev/md127 /dev/md127: HDIO_DRIVE_CMD(identify) failed: Inappropriate ioctl for device Thanks, Mike
Hi Mike, My BIOS RAID array is an Intel Software RAID on a ASUS P8H67-M PRO/CSM motherboard. Output from requested command is: $ sudo hdparm -i /dev/mapper/isw_efjbbijhh_Vol0 /dev/mapper/isw_efjbbijhh_Vol0: HDIO_DRIVE_CMD(identify) failed: Inappropriate ioctl for device HDIO_GET_IDENTITY failed: Inappropriate ioctl for device $ The /dev/mapper/isw_efjbbijhh_Vol0 RAID mirror is also known as /dev/dm-0: $ sudo hdparm -i /dev/dm-0 /dev/dm-0: HDIO_DRIVE_CMD(identify) failed: Inappropriate ioctl for device HDIO_GET_IDENTITY failed: Inappropriate ioctl for device $ As far as I can tell the Motherboard BIOS RAID does not have a serial number. Also when I run "gsmartcontrol", the RAID does not show up as a separate device. The two member drives do show up though as regular drives (/dev/sdc and /dev/sdd in my case). Curtis
Created attachment 305916 [details] [review] Display device serial numbers (v1) Hi Curtis, Here's the patch for this. Chose to just use hdparm as it keeps the code simpler, it's to most well known and popular tool for the task and included by default in some distros, including GParted Live CD and System Rescue CD. (Doesn't seem to be included in recent Fedora's by default though). Performance on my desktop with 4 hard drives, with 2 Linux Software RAID Arrays on top (therefore running hdparm -I 6 times) adds 0.04 seconds to the device refresh time. 0.001663 + 0.003069 + 0.012383 + 0.003516 + 0.008544 + 0.009747 = 0.38922 Fragment of timed logging for this. 17.879664 +14.771361 STOPWATCH_RESET 0.000000 +0.000000 set_devices_thread() (pdevices) 0.000065 +0.000065 set_devices_thread() Refreshing caches ... 0.286195 +0.001540 set_devices_thread() Scanning for devices (aka disks) ... 0.292192 +0.005997 set_devices_thread() Scanning devices (aka disks) for partitions and contents ... /dev/md126: unrecognised disk label 0.294930 +0.002738 set_device_serial_number() /dev/md126 0.294953 +0.000023 execute_command() execute: hdparm -I /dev/md126 0.296615 +0.001663 execute_command() exit status 0 0.296668 +0.000052 set_device_serial_number() serial_number="none" /dev/md127: unrecognised disk label 0.299798 +0.003130 set_device_serial_number() /dev/md127 0.299816 +0.000018 execute_command() execute: hdparm -I /dev/md127 0.302885 +0.003069 execute_command() exit status 0 0.302944 +0.000060 set_device_serial_number() serial_number="none" 0.382538 +0.079594 set_device_serial_number() /dev/sda 0.382572 +0.000034 execute_command() execute: hdparm -I /dev/sda 0.394955 +0.012383 execute_command() exit status 0 0.395073 +0.000118 set_device_serial_number() serial_number="S1WFJDSZ123732" ... 1.464694 +0.032995 set_device_serial_number() /dev/sdb 1.464728 +0.000034 execute_command() execute: hdparm -I /dev/sdb 1.468244 +0.003516 execute_command() exit status 0 1.468370 +0.000126 set_device_serial_number() serial_number="DCF4300940SE940B4507" ... 2.719533 +0.000749 set_device_serial_number() /dev/sdc 2.719549 +0.000016 execute_command() execute: hdparm -I /dev/sdc 2.728093 +0.008544 execute_command() exit status 0 2.728222 +0.000128 set_device_serial_number() serial_number="S2R8J9EC617288" /dev/sdd: unrecognised disk label 2.729791 +0.001570 set_device_serial_number() /dev/sdd 2.729811 +0.000020 execute_command() execute: hdparm -I /dev/sdd 2.739557 +0.009747 execute_command() exit status 0 2.739666 +0.000108 set_device_serial_number() serial_number="S2TBJA0C137217" 2.740099 +0.000433 set_devices_thread() return Thanks, Mike
Created attachment 305954 [details] [review] Display device serial numbers (v2) Here's patch v2. The only difference is changing the patch summary from: Display device serial number (#751251) to: Display device serial numbers (#751251)
Hi Mike, I am testing with patch set v2 on my physical computer with kubuntu 12.04, and GParted gets stuck "Searching /dev/sdf partitions". The output on the command line follows: $ sudo src/gpartedbin ====================== libparted : 2.3 ====================== (gpartedbin:10261): glibmm-CRITICAL **: unhandled exception (type Glib::Error) in signal handler: domain: g_convert_error code : 1 what : Invalid byte sequence in conversion input $ Device /dev/sdf is my Kobo Touch eReader which I happened to have connected at the time and has no partitions. Note that GParted 0.22.0 works fine with this device. More information follows: $ sudo blkid | grep sdf /dev/sdf: LABEL="KOBOeReader" UUID="5091-AB25" TYPE="vfat" $ sudo parted /dev/sdf unit s print Model: Kobo eReader-2.5.2 (scsi) Disk /dev/sdf: 2874814s Sector size (logical/physical): 512B/512B Partition Table: loop Number Start End Size File system Flags 1 0s 2874813s 2874814s fat32 $ sudo hdparm -i /dev/sdf /dev/sdf: SG_IO: bad/missing sense data, sb[]: 70 00 05 00 00 00 00 0a 00 00 00 00 24 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 HDIO_GET_IDENTITY failed: Invalid argument $ On a more minor note (more of a personal preference), I think the last changed line of patch v2 would look better if the spacing matched the surrounding code. E.g., in Win_GParted::File_Label_Device_info() device_info[ t++ ] ->set_text( devices[ current_device ] .model ) ; device_info[ t++ ] ->set_text( devices[current_device].serial_number ) ; device_info[ t++ ] ->set_text( Utils::format_size( devices[ current_device ] .length, devices[ current_device ] .sector_size ) ) ; If you need any more information just let me know. Thanks, Curtis
You can get the serial number by grepping /dev/disk/by-id Every Linux distribution uses udev. You can also use lsblk from util-linux which is also a standard. lsblk /dev/sda -o SERIAL Just tossing this out there as an option if hdparm doesn't work out.
Hi Curtis, Can you run the following for me and send me the 2 files and report the exit status. From this I will be able to fake your hdparm result and debug the error. sudo su - hdparm -I /dev/sdf 1> hdparm.out 2> hdparm.err echo $? exit Notes: 1) Becoming root first, then running hdparm I am not 100% sure sudo returns the command exit status unmodified. 2) Please use capital I option to hdparm for this one. Lower case i does a slightly different hdparm action. See the man page for details. Thanks, Mike
Created attachment 306037 [details] Tarball of hdparm.out and hdparm.err Hi Mike, Following are the relevant commands I ran: # hdparm -I /dev/sdf 1> hdparm.out 2> hdparm.err root@octo:~# echo $? 0 # tar cvf hdparm.tar.gz ./hdparm.out ./hdparm.err ./hdparm.out ./hdparm.err # Curtis
Thanks Patrick for the tips. I found my Kobo Touch eReader in the file: /dev/disk/by-id/usb-Kobo_eReader-2.5.2_KG31C0402421B-0\:0 which appears to be a symbolic link to ../../sdf Since this is /dev/sdf, grepping this file would in theory scan the entire device. I also ran the lsblk command (on Kubuntu 12.04) and it appears that the SERIAL column is not supported in this older version. # lsblk /dev/sdf -o SERIAL lsblk: unknown column: SERIAL # lsblk --help Usage: lsblk [options] [<device> ...] Options: -a, --all print all devices -b, --bytes print SIZE in bytes rather than in human readable format -d, --nodeps don't print slaves or holders -D, --discard print discard capabilities -e, --exclude <list> exclude devices by major number (default: RAM disks) -f, --fs output info about filesystems -h, --help usage information (this) -i, --ascii use ascii characters only -m, --perms output info about permissions -l, --list use list format ouput -n, --noheadings don't print headings -o, --output <list> output columns -P, --pairs use key="value" output format -r, --raw use raw output format -t, --topology output info about topology Available columns: NAME device name KNAME internal kernel device name MAJ:MIN major:minor device number FSTYPE filesystem type MOUNTPOINT where the device is mounted LABEL filesystem LABEL UUID filesystem UUID RO read-only device RM removable device MODEL device identifier SIZE size of the device STATE state of the device OWNER user name GROUP group name MODE device node permissions ALIGNMENT alignment offset MIN-IO minimum I/O size OPT-IO optimal I/O size PHY-SEC physical sector size LOG-SEC logical sector size ROTA rotational device SCHED I/O scheduler name RQ-SIZE request queue size TYPE device type DISC-ALN discard alignment offset DISC-GRAN discard granularity DISC-MAX discard max bytes DISC-ZERO discard zeroes data For more information see lsblk(8). #
Hi Patrick, Thank your for the suggestions. Unfortunately after initial investigations there are limitations with using lsblk and /dev/disk/by-id which makes them somewhat complicated to use. So far I think it best that we stick with using hdparm. It works the same everywhere it is available and it is easy to say that if the serial number is not show, hdparm is not installed. Thanks, Mike lsblk -o SERIAL --------------- I had previously discounted lsblk as the SERIAL column was only recently added and many of the distributions we want to support don't have it, including Ubuntu 14.04 LTS. It does have the advantage of always being installed, where as I have found that hdparm may not be. Just tried lsblk -o KNAME,SERIAL on GParted Liave CD 0.21.0 and System Rescue CD 4.5.2 (the 2 live CDs I have to hand) and neither of them report a serial number for the devices with serial numbers. root@debian:~# lsblk -V lsblk from util-linux 2.25.2 root@debian:~# lsblk -o KNAME,SERIAL sda sda1 sda2 sdb sdc sr0 root@debian:~# hdparm -I /dev/sda | egrep 'Model|Serial' Model Number: VBOX HARDDISK Serial Number: VB6a6cf851-9acc0194 /dev/disk/by-id --------------- Using /dev/disk/by-id/ directory entries populated by uder appears to work all the way back to the oldest distribution we attempt to support (CentOS 5, first released in 2007). It does use underscores to separate contatenated fields though so if any serial numbers include underscores we would extract the wrong value, whereas for hdparm the serial number could contain underscores and spaces and we could still extract it correctly. Some distros also present multiple different names in /dev/disk/by-id/ directory to select from. Found in CentOS 6 and System Rescue CD 4.5.2 so far. # ls -l /dev/disk/by-id | fgrep -v -- -part | egrep 'sda|sdb|md127' lrwxrwxrwx. 1 root root 9 Jun 21 10:28 ata-SAMSUNG_HM500JI_S1WFJDSZ123732 -> ../../sda lrwxrwxrwx. 1 root root 9 Jun 21 10:28 ata-SAMSUNG_SSD_UM410_Series_2.5__8GB_DCF4300940SE940B4507 -> ../../sdb lrwxrwxrwx. 1 root root 11 Jun 24 13:56 md-name-rockover:1 -> ../../md127 lrwxrwxrwx. 1 root root 11 Jun 24 13:56 md-uuid-9807e405:7950c7b2:84876896:39bcd7f9 -> ../../md127 lrwxrwxrwx. 1 root root 9 Jun 21 10:28 scsi-SATA_SAMSUNG_HM500JIS1WFJDSZ123732 -> ../../sda lrwxrwxrwx. 1 root root 9 Jun 21 10:28 scsi-SATA_SAMSUNG_SSD_UM4DCF4300940SE940B4507 -> ../../sdb lrwxrwxrwx. 1 root root 9 Jun 21 10:28 wwn-0x50000f0056414431 -> ../../sdb lrwxrwxrwx. 1 root root 9 Jun 21 10:28 wwn-0x50024e9002d55699 -> ../../sda
Hi Curtis, I've looked at the hdparm.out from comment #10. The serial number reported by "hdparm -I" is full of binary data. So I am pretty certain that copying it into a Glib::ustring type will be causing the error because it isn't valid a UTF-8 string. The question is how to handle this. Mike
Hi Mike, What do you think of setting the serial number to nothing (e.g. "") if "hdparm -I" returns binary data that is invalid for a UTF-8 string? My thoughts are that valid serial numbers will not contain invalid UTF-8 characters. Curtis
I was thinking along similar lines, limiting the serial number to printable ASCII characters using isprint(). I have reproduced the hang and and tracked it down to never returning from this IOChannel::read() call when the unhandled invalid byte sequence exception is thrown: src/PipeCapture.cc 49 bool PipeCapture::OnReadable( Glib::IOCondition condition ) 50 { ... 58 Glib::ustring str; >> 59 Glib::IOStatus status = channel->read( str, 512 ); 60 if (status == Glib::IO_STATUS_NORMAL) 61 { 62 for( Glib::ustring::iterator s = str.begin(); s != str.end(); s++ ) This is when GParted is reading the output or error stream from the executable. Found this in the IOChannel class reference: https://developer.gnome.org/glibmm/stable/classGlib_1_1IOChannel.html Note that IOChannels implement an automatic implicit character set conversion to the data stream, and usually will not pass by default binary data unchanged. To set the encoding of the channel, use e.g. set_encoding("ISO-8859-15"). To set the channel to no encoding, use set_encoding() without any arguments. I'll see where I get with setting the encoding to "" so it can accept binary data.
I have also had a further look at the names populated by udev in /dev/disk/by-id/. Short summary is there is significant variation to deal with between different versions of what I assume are udev rule sets creating different entries on different distributions and device types. I still think using hdparm is the best choice if we can make it work. CentOS 5 only have a single "scsi-" prefix reference to hard drives. # ls -l /dev/disk/by-id | cut -c40- ata-VBOX_CD-ROM_VB2-01700376 -> ../../hdc scsi-SATA_VBOX_HARDDISK_VB4f3565dc-43b26a89 -> ../../sdc scsi-SATA_VBOX_HARDDISK_VB53d72be2-8ad094a4 -> ../../sda scsi-SATA_VBOX_HARDDISK_VB8ccb627f-f7c01cc1 -> ../../sdd scsi-SATA_VBOX_HARDDISK_VBc0a3fb77-3a7e5117 -> ../../sdb Most distributions use "ata-" prefix to hard drives and have partition specific entry with "-partNN" suffixes to ignore. E.g. From Ubuntu 14.04 LTS. # ls -l /dev/disk/by-id | cut -c40- ata-VBOX_CD-ROM_VB2-01700376 -> ../../sr0 ata-VBOX_HARDDISK_VBae6db85f-575214d5 -> ../../sdb ata-VBOX_HARDDISK_VBae6db85f-575214d5-part1 -> ../../sdb1 ata-VBOX_HARDDISK_VBfd6e48d1-b89a5170 -> ../../sda ata-VBOX_HARDDISK_VBfd6e48d1-b89a5170-part1 -> ../../sda1 ata-VBOX_HARDDISK_VBfd6e48d1-b89a5170-part2 -> ../../sda2 ata-VBOX_HARDDISK_VBfd6e48d1-b89a5170-part5 -> ../../sda5 On my CentOS 6 desktop I have found that it contains entries for disks starting with "ata-", "scsi-" and "wwn-". Also the "scsi-" entry has had the model truncated and not separated with an underscore from the serial number, so it extracter serial number would be wrong. # hdparm -I /dev/sdc | egrep 'Model Number|Serial Number' Model Number: ST1000LM024 HN-M101MBB Serial Number: S2R8J9EC617288 # ls -l /dev/disk/by-id | fgrep sdc | cut -d' ' -f10- ata-ST1000LM024_HN-M101MBB_S2R8J9EC617288 -> ../../sdc scsi-SATA_ST1000LM024_HN-S2R8J9EC617288 -> ../../sdc wwn-0x50004cf207c5ea67 -> ../../sdc Again on my CentOS 6 desktop, USB attached hard drives report the serial number of the embedded USB controller in the device, which is different to the serial number of the hard drive itself. The serial number has "-0:0" appended which is probably part of the location of the device within the USB bus. # hdparm -I /dev/sdg | egrep 'Model Number|Serial Number' Model Number: WDC WD2500BEVT-22ZCT0 Serial Number: WD-WXE409R30341 # ls -l /dev/disk/by-id | fgrep sdg | cut -c41- usb-BUFFALO_HD-PEU2_57442D575845343039523330-0:0 -> ../../sdg usb-BUFFALO_HD-PEU2_57442D575845343039523330-0:0-part1 -> ../../sdg1 # lshw ... *-usb:2 description: Mass storage device product: HD-PEU2 vendor: BUFFALO ... serial: 57442D575845343039523330 ... *-disk description: SCSI Disk product: HD-PEU2 vendor: BUFFALO physical id: 0.0.0 bus info: scsi@13:0.0.0 logical name: /dev/sdg version: 1.03 serial: WD-WXE409R30341 ... So what we would need to do would be approximately: 1) Ignore -partN names 2) Match link target with device names 3) Use ata- names if available 4) Use scsi- names as fallback 5) Optionally use usb- names as second fallback Drop trailing -N:N (USB bus location reference) 6) Take serial number from last underscore to the end Hope this gets the serial number. If using usb- name it won't get the serial number of the hard drive, but instead the serial number of the embedded USB controller.
Created attachment 306190 [details] [review] Display device serial numbers (v3) Hi Curtis, Here's patchset v3. Changes from v2 in comment #6: P1 - Slightly adjusts the logic of parsing the hdparm output to get the serial number. - Changes white space in line added to ::File_Label_Device_info() as preferred. - Now testing timing with even more devices. See below. P2 - New patch which calls IOChannel::set_encoding("") to disable character set conversion prevent binary data causing GParted to never complete the scanning phase. Now I'm testing with even more devices attached. 4 internal hard drives, 2 Linux software RAID arrays on those hard drives, 2 USB flash drives and 1 USB hard drive. Running hdparm 9 times to try to get all those serial numbers adds 0.07 seconds to the device refresh time. 0.005993 + 0.002872 + 0.012937 + 0.004319 + 0.009536 + 0.010463 + 0.005259 + 0.006374 + 0.015846 = 0.073599 Fragment of timed logging for this: 67.160988 +60.923983 STOPWATCH_RESET 0.000000 +0.000000 set_devices_thread() (pdevices) 0.000068 +0.000068 set_devices_thread() Refreshing caches ... 0.279787 +0.001751 set_devices_thread() Scanning for devices (aka disks) ... 0.298233 +0.018446 set_devices_thread() Scanning devices (aka disks) for partitions and contents ... /dev/md126: unrecognised disk label 0.303283 +0.005050 set_device_serial_number() /dev/md126 0.303329 +0.000046 execute_command() execute: hdparm -I /dev/md126 0.309322 +0.005993 execute_command() exit status 0 0.309356 +0.000034 set_device_serial_number() serial_number="none" /dev/md127: unrecognised disk label 0.321883 +0.012527 set_device_serial_number() /dev/md127 0.321914 +0.000031 execute_command() execute: hdparm -I /dev/md127 0.324786 +0.002872 execute_command() exit status 0 0.324813 +0.000027 set_device_serial_number() serial_number="none" 0.546797 +0.221984 set_device_serial_number() /dev/sda 0.546832 +0.000035 execute_command() execute: hdparm -I /dev/sda 0.559769 +0.012937 execute_command() exit status 0 0.559896 +0.000128 set_device_serial_number() serial_number="S1WFJDSZ123732" ... 1.632723 +0.038783 set_device_serial_number() /dev/sdb 1.632779 +0.000055 execute_command() execute: hdparm -I /dev/sdb 1.637097 +0.004319 execute_command() exit status 0 1.637131 +0.000033 set_device_serial_number() serial_number="DCF4300940SE940B4507" ... 2.889022 +0.000832 set_device_serial_number() /dev/sdc 2.889044 +0.000022 execute_command() execute: hdparm -I /dev/sdc 2.898580 +0.009536 execute_command() exit status 0 2.898721 +0.000141 set_device_serial_number() serial_number="S2R8J9EC617288" /dev/sdd: unrecognised disk label 2.900320 +0.001598 set_device_serial_number() /dev/sdd 2.900337 +0.000018 execute_command() execute: hdparm -I /dev/sdd 2.910800 +0.010463 execute_command() exit status 0 2.910937 +0.000136 set_device_serial_number() serial_number="S2TBJA0C137217" 2.930277 +0.019340 set_device_serial_number() /dev/sde 2.930308 +0.000032 execute_command() execute: hdparm -I /dev/sde 2.935567 +0.005259 execute_command() exit status 0 2.935610 +0.000043 set_device_serial_number() serial_number="none" ... 3.980598 +0.025127 set_device_serial_number() /dev/sdf 3.980648 +0.000050 execute_command() execute: hdparm -I /dev/sdf 3.987022 +0.006374 execute_command() exit status 0 3.987057 +0.000035 set_device_serial_number() serial_number="none" ... 5.031146 +0.005163 set_device_serial_number() /dev/sdg 5.031179 +0.000033 execute_command() execute: hdparm -I /dev/sdg 5.047025 +0.015846 execute_command() exit status 0 5.047149 +0.000124 set_device_serial_number() serial_number="WD-WXE409R30341" ... 6.066192 +0.000055 set_devices_thread() return Thanks, Mike
Hi Mike, Patch set v3 from comment #17 now works with my Kobo Touch eReader. :-) Thank you for triaging and fixing this problem. Since the change to IOChannel::set_encoding("") to disable character set conversion might have more global consequences, I plan to do more testing. In particular I am curious as to what effect this change might have on non-English file system labels. Curtis
Hi Mike, When testing on a system with no hdparm command (e.g., mv /sbin/hdparm /sbin/hdparm.orig), patch set v3 tries to run the hdparm command for each device and an error message is reported on stderr (as you pointed out in the commit message). What do you think of testing for hdparm first before running the command which generates multiple stderr messages? We have implemented such "test command exists before running" logic in FS_Info and LVM_PV_Info. Perhaps we could implement a Device_Info? What are your thoughts? Curtis
Hi Curtis, I just executed hdparm, without checking first, because it seemed wasteful to run Glib::find_program_in_path("hdparm") multiple times for every refresh. And failure to execute the command case is implicitly handled in the parsing code. You're correct that having avoidable errors written to stderr is bad practice. I intend to change the code to check if hdparm exists once and remember the result. Then also make dialog View > File System Support, button [Rescan For Supported Actions] re-check, just as it does for file system specific commands. Mike Stracing GParted finds that calling Glib::find_program_in_path("hdparm") does the following OS calls. The Linux VFS is very fast, but it would be better to remember the result than repeatedly check for the same thing. access("/usr/lib64/qt-3.3/bin/hdparm", X_OK) = -1 ENOENT (No such file or directory) access("/usr/local/sbin/hdparm", X_OK) = -1 ENOENT (No such file or directory) access("/usr/local/bin/hdparm", X_OK) = -1 ENOENT (No such file or directory) access("/sbin/hdparm", X_OK) = 0 getuid() = 0 stat("/sbin/hdparm", {st_mode=S_IFREG|0755, st_size=137, ...}) = 0 stat("/sbin/hdparm", {st_mode=S_IFREG|0755, st_size=137, ...}) = 0
Your plan sounds like a good improvement. I agree that checking once on gparted startup is all that is needed. Your suggestion to also check on [Rescan For Supported Actions] is one I like too. :-) My main concern with the "missing hdparm" error message was to reduce the support requests that might be raised as a result of users seeing this message. Ideally we want to make our users as self-sufficient as reasonably possible, and to reduce future support requests.
Created attachment 306431 [details] [review] Display device serial numbers (v4) Hi Curtis, Here's patchset v4. Makes the agreed changes. Details of those changes: P1 "Display device serial numbers" - Searches the path each time before executing hdparm to avoid reporting command not found errors. P2 "Don't hang reading binary data from command output" (no changes) P3 "Remember result of searching the PATH for the hdparm command" - New patch to avoid repeated checking. P4 "Remember result of searching the PATH for udevadm and udevsettle cmds" - New patch extending to the udevadm and udevsettle commands too. Thanks, Mike
Thank you Mike for all your improvements to the patch set for this report. This latest patch set works great for me. I tested on kubuntu 12.04 and also the most recent debian 9 stretch testing. I hope you don't mind, but I noticed the word "the" doubled in the first patch commit comment, so I took the liberty of removing the extra word. Patch set v4 in comment #22 has been committed to the git repository for inclusion in the next release of GParted. The relevant git commits can be viewed at the following links: Display device serial numbers (#751251) https://git.gnome.org/browse/gparted/commit/?id=4b72ecd44eba71b055f939fa5eff4572bb480ad9 Don't hang reading binary data from command output (#751251) https://git.gnome.org/browse/gparted/commit/?id=4fce7cd5eed5b298215b57dc9b1ffd1aff2fe22a Remember result of searching the PATH for the hdparm command (#751251) https://git.gnome.org/browse/gparted/commit/?id=54d0e3d056f42190d9cd688d07c23161d383a82f Remember result of searching the PATH for udevadm and udevsettle cmds https://git.gnome.org/browse/gparted/commit/?id=038209a9cf0f08915d2c30271a1e5e3ac8198a84
This enhancement was included in the GParted 0.23.0 release on August 3, 2015.