Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The 'sensors -s' command in docker-platform-monitor rewrite incorrect set statements by default config. #1055

Closed
wadelnn opened this issue Oct 19, 2017 · 1 comment
Assignees

Comments

@wadelnn
Copy link
Contributor

wadelnn commented Oct 19, 2017

Description

The "sensors -s" will load platform specific config (/etc/sensors.d/sensors.conf) first. And then load the default config (/etc/sensors3.conf). But some set statements will rewrite by default config. When use coammand sensors and it will show incorrect value or label.

Steps to reproduce the issue:

  1. modify /usr/share/sonic/device/x86_64-ingrasys_s9100-r0/sensors.conf and search chip "w83795adg-*"
  2. Add test code under chip "w83795adg-*" as follows:
set in12_min 3.3 * 0.95
set in12_max 3.3 * 1.05
  1. Save the file and reboot
  2. Login and execute command sensors

Describe the results you received:
The 3.3V min and max value still between 3.3 * 0.90 and 3.3 * 1.10

Describe the results you expected:
The 3.3V min value is 3.3 * 0.95 and max value is 3.3 * 1.05

Additional information you deem important (e.g. issue happens only occasionally):

I think this issue is related by this code:
https://github.com/Azure/sonic-buildimage/blob/f49cac086f8421bf5d76a4798c4906700a322a50/dockers/docker-platform-monitor/lm-sensors.sh#L6

We fixed this issue by replace lm-sensors.sh line 6 as follows:

if [ -e /etc/sensors.d/sensors.conf ]; then
    /usr/bin/sensors -s -c /etc/sensors.d/sensors.conf > /dev/null 2>&1
else
    /usr/bin/sensors -s > /dev/null 2>&1
fi

And this code will avoid rewrite default config.

@jleveque
Copy link
Contributor

jleveque commented Oct 20, 2017

I was able to confirm this on a different platform. This appears to be a bug in lm-sensors. Thanks for catching this.

Looking at sensors -s with strace, the config files appear to get loaded in the proper order, yet the default values do not appear to get overridden by the platform-specific ones.

open("/etc/sensors3.conf", O_RDONLY)    = 3
ioctl(3, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, 0x7ffd70c2ef80) = -1 ENOTTY (Inappropriate ioctl for device)
fstat(3, {st_mode=S_IFREG|0644, st_size=10344, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fd940d57000
read(3, "# libsensors configuration file\n"..., 8192) = 8192
read(3, " label in4 \"+12V\"\n    label in5 "..., 8192) = 2152
read(3, "", 4096)                       = 0
read(3, "", 8192)                       = 0
ioctl(3, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, 0x7ffd70c2d910) = -1 ENOTTY (Inappropriate ioctl for device)
close(3)                                = 0
munmap(0x7fd940d57000, 4096)            = 0
openat(AT_FDCWD, "/etc/sensors.d", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
getdents(3, /* 4 entries */, 32768)     = 112
getdents(3, /* 0 entries */, 32768)     = 0
close(3)                                = 0
stat("/etc/sensors.d/sensors.conf", {st_mode=S_IFREG|0644, st_size=1449, ...}) = 0
open("/etc/sensors.d/sensors.conf", O_RDONLY) = 3
ioctl(3, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, 0x7ffd70c2de70) = -1 ENOTTY (Inappropriate ioctl for device)
fstat(3, {st_mode=S_IFREG|0644, st_size=1449, ...}) = 0
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7fd940d57000
read(3, "# libsensors configuration file\n"..., 8192) = 1449
read(3, "", 4096)                       = 0
read(3, "", 8192)                       = 0
ioctl(3, SNDCTL_TMR_TIMEBASE or SNDRV_TIMER_IOCTL_NEXT_DEVICE or TCGETS, 0x7ffd70c2c800) = -1 ENOTTY (Inappropriate ioctl for device)
close(3)                                = 0
munmap(0x7fd940d57000, 4096)            = 0

I am creating a pull request using your workaround for the time being.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants