[lm-sensors] [PATCH] avoid unwanted sensors-detect termination when the /dev/port is missing

[lm-sensors] [PATCH] avoid unwanted sensors-detect termination when the /dev/port is missing

[Jaromir Capik]

[-- Attachment #1: Type: text/plain, Size: 680 bytes --]

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: jcapik@redhat.com
Web: www.cz.redhat.com
Red Hat Czech s.r.o., Purkynova 99/71, 612 45, Brno, Czech Republic
IC: 27690016 

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sensors-detect-no-dev-port.patch --]
[-- Type: text/x-patch; name=sensors-detect-no-dev-port.patch, Size: 2243 bytes --]

--- sensors-detect.orig	2013-01-17 16:04:10.000000000 +0100
+++ sensors-detect	2013-01-17 16:05:32.209400430 +0100
@@ -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";
 	}

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] avoid unwanted sensors-detect termination when the /dev/port is missing

[Jean Delvare]

Hi Jaromir,

On Thu, 17 Jan 2013 10:12:41 -0500 (EST), Jaromir Capik wrote:
> 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.

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.

> 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.

> 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

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] avoid unwanted sensors-detect termination when the /dev/port is missing

[Jaromir Capik]

> Hi Jaromir,
> 

Hi Jean.

> On Thu, 17 Jan 2013 10:12:41 -0500 (EST), Jaromir Capik wrote:
> > 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.
> 
> 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?

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.


> I am not particularly interested in tweaking sensors-detect for
> theoretical-only cases.
> 
> > 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.

Yes, that sounds better. I'm not against any future
modifications. That was just the kick off. 


> 
> > 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.


> 
> [1] http://www.lm-sensors.org/changeset/6116
> 
> --
> Jean Delvare
> 

Thank you.

Regards,
Jaromir.

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] [PATCH] avoid unwanted sensors-detect termination when the /dev/port is missing

[Jean Delvare]

On Wed, 23 Jan 2013 07:17:34 -0500 (EST), Jaromir Capik wrote:
> > 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?
> 
> 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.

OK, thanks for the information. I do agree about flexibility, as long
as is is needed and not "just because we can".

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors