avoid unwanted sensors-detect termination when the /dev/port is missing
PATCH 1/1 avoid unwanted sensors-detect termination when the /dev/port is missing
Hello guys.
The device file /dev/port might be missing in some cases and the sensors detection is terminated when the user tries to detect sensors dependent on it's existence. That's not correct -> it's not a reason for terminating the detection.
The attached patch solves the issue, so that a warning is displayed and the detection continues.
The patch can be applied on the current trunk version. Please, check the patch and merge it if possible.
Thank you.
Regards,
Jaromir.
--
Jaromir Capik
Red Hat Czech, s.r.o.
Software Engineer / BaseOS
Email: [email protected]
Web: www.cz.redhat.com
Red Hat Czech s.r.o., Purkynova 99/71, 612 45, Brno, Czech Republic
IC: 27690016
sensors-detect renamed from sensors-detect.orig +26 -19
@@ -2471,9 +2471,12 @@ sub initialize_ioports { - sysopen(IOPORTS, "/dev/port", O_RDWR) - or die "/dev/port: $!\n"; - binmode(IOPORTS); + if (sysopen(IOPORTS, "/dev/port", O_RDWR)) { + binmode(IOPORTS); + return 1; + } + print "/dev/port: $!\n"; + return 0; } sub close_ioports
@@ -3511,13 +3514,14 @@ print("Can't set I2C address for $dev\n"), next; - initialize_ioports(); - $alias_detect = $detected->[$isa]->{alias_detect}; - $is_alias = &$alias_detect($detected->[$isa]->{isa_addr}, - \*FILE, - $detected->[$i2c]->{i2c_addr}); + if (initialize_ioports()) { + $alias_detect = $detected->[$isa]->{alias_detect}; + $is_alias = &$alias_detect($detected->[$isa]->{isa_addr}, + \*FILE, + $detected->[$i2c]->{i2c_addr}); + close_ioports(); + } close(FILE); - close_ioports(); next unless $is_alias; # This is an alias: copy the I2C data into the ISA
@@ -6819,10 +6823,11 @@ "standard I/O ports to probe them. This is usually safe.\n"; print "Do you want to scan for Super I/O sensors? (YES/no): "; unless (<STDIN> =~ /^\s*n/i) { - initialize_ioports(); - $superio_features |= scan_superio(0x2e, 0x2f); - $superio_features |= scan_superio(0x4e, 0x4f); - close_ioports(); + if (initialize_ioports()) { + $superio_features |= scan_superio(0x2e, 0x2f); + $superio_features |= scan_superio(0x4e, 0x4f); + close_ioports(); + } } print "\n";
@@ -6835,9 +6840,10 @@ "interfaces? (YES/no): "; unless (<STDIN> =~ /^\s*n/i) { if (!ipmi_from_smbios()) { - initialize_ioports(); - scan_isa_bus(\@ipmi_ifs); - close_ioports(); + if (initialize_ioports()) { + scan_isa_bus(\@ipmi_ifs); + close_ioports(); + } } } print "\n";
@@ -6851,9 +6857,10 @@ $input = <STDIN>; unless ($input =~ /^\s*n/i || ($superio_features && $input !~ /^\s*y/i)) { - initialize_ioports(); - scan_isa_bus(\@chip_ids); - close_ioports(); + if (initialize_ioports()) { + scan_isa_bus(\@chip_ids); + close_ioports(); + } } print "\n"; }
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Hi Jaromir,
Quoted 7 lines · Jaromir Capik
Hello guys.
The device file
/dev/portmight be missing in some cases and the sensors detection is terminated when the user tries to detect sensors dependent on it's existence. That's not correct -> it's not a reason for terminating the detection.
I admit that dying in this case is a little abrupt. That being said, without /dev/port, you won't be able to detect Super-I/O chips, ISA chips and I/O-based IPMI interfaces, which only leave you with I2C/SMBus-based, PCI-based and MSR-based sensors. Are there really systems out there without /dev/proc where any of these is present? I am not particularly interested in tweaking sensors-detect for theoretical-only cases.
Quoted 2 lines
The attached patch solves the issue, so that a warning is displayed and the detection continues.
This is a first step forward, but from a user-friendliness perspective I'd say it is insufficient. If /dev/port isn't there then you shouldn't even try to open it, even less try 4 times and display 4 times the same error message. Instead, you should check for /dev/port absence upfront, let the user know about that and the limitations it implies, and then plain skip all detection steps which you know have no chance of succeeding.
Quoted 2 lines
The patch can be applied on the current trunk version. Please, check the patch and merge it if possible.
I have applied it [1], with an additional STDERR for the error message destination.
[1] http://www.lm-sensors.org/changeset/6116
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 2 lines
Hi Jaromir,
Hi Jean.
Quoted 9 lines · Jaromir Capik
Quoted 7 lines
Hello guys.
The device file
/dev/portmight be missing in some cases and the sensors detection is terminated when the user tries to detect sensors dependent on it's existence. That's not correct -> it's not a reason for terminating the detection.
Quoted 5 lines
I admit that dying in this case is a little abrupt. That being said, without
/dev/port, you won't be able to detect Super-I/O chips, ISA chips and I/O-based IPMI interfaces, which only leave you with I2C/SMBus-based, PCI-based and MSR-based sensors. Are there really systems out there without/dev/procwhere any of these is present?
Yes, such systems exist. I had the pleasure to debug the issue directly on 2 systems where the /dev/port was missing, but I2C bus was present. One of the systems even didn't have any PCI bus. And I believe that we can expect more hardware like that in the future. With ARM based boards even more oddities can appear. The detection script needs to be as flexible as possible to handle such situations.
Quoted 6 lines
I am not particularly interested in tweaking sensors-detect for theoretical-only cases.
Quoted 2 lines
The attached patch solves the issue, so that a warning is displayed and the detection continues.
Quoted 11 lines
This is a first step forward, but from a user-friendliness perspective
I'd say it is insufficient. If/dev/portisn't there then you shouldn't
even try to open it, even less try 4 times and display 4 times the same
error message. Instead, you should check for/dev/portabsence upfront,
let the user know about that and the limitations it implies, and then plain skip all detection steps which you know have no chance of succeeding.
Yes, that sounds better. I'm not against any future modifications. That was just the kick off.
Quoted 7 lines
Quoted 3 lines
The patch can be applied on the current trunk version. Please, check the patch and merge it if possible.
I have applied it [1], with an additional STDERR for the error message
destination.
Oh yes, you're right. It needs to go to STDERR.
Quoted 6 lines
[1] http://www.lm-sensors.org/changeset/6116
--
Jean Delvare
Thank you.
Regards,
Jaromir.
_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
Quoted 6 lines · Jaromir Capik
Quoted 5 lines
I admit that dying in this case is a little abrupt. That being said, without
/dev/port, you won't be able to detect Super-I/O chips, ISA chips and I/O-based IPMI interfaces, which only leave you with I2C/SMBus-based, PCI-based and MSR-based sensors. Are there really systems out there without/dev/procwhere any of these is present?
Quoted 7 lines
Yes, such systems exist. I had the pleasure to debug the issue directly on 2 systems where the
/dev/portwas missing, but I2C bus was present. One of the systems even didn't have any PCI bus. And I believe that we can expect more hardware like that in the future. With ARM based boards even more oddities can appear. The detection script needs to be as flexible as possible to handle such situations.
OK, thanks for the information. I do agree about flexibility, as long as is is needed and not "just because we can".
Signature · 6 lines
Jean Delvare _______________________________________________ lm-sensors mailing list [email protected] http://lists.lm-sensors.org/mailman/listinfo/lm-sensors