All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: lm-sensors@vger.kernel.org
Subject: Re: [lm-sensors] Updated initialization script, testers wanted
Date: Tue, 13 Jan 2009 18:45:58 +0000	[thread overview]
Message-ID: <496CE166.5060803@redhat.com> (raw)
In-Reply-To: <20090112140236.6b74302e@hyperion.delvare>



Jean Delvare wrote:
> Hi Hans,
> 
> On Tue, 13 Jan 2009 09:02:33 +0100, Hans de Goede wrote:
>> Jean Delvare wrote:
>>> I have implemented and committed the sensors-detect side of ticket
>>> #2246. Now the lm_sensors initialization script needs to be updated as
>>> well, to make use of the new configuration file syntax. The
>>> initialization script in our repository is meant for Red Hat
>>> distributions but I don't have any, so I can't test my changes. The
>>> patch I intend to apply is attached, could you (or anyone with a Red
>>> Hat distribution) give it a try and confirm that it works OK?
>>> Beforehand, you'll need to download and run the latest version of
>>> sensors-detect:
>>>
>>> http://www.lm-sensors.org/svn/lm-sensors/branches/lm-sensors-3.0.0/prog/detect/sensors-detect
>>>
>>> and let it regenerate /etc/sysconfig/lm_sensors.
>> Erm,
>>
>> Not tested, I'm not so happy with this change. I've read the ticker trying to 
>> find out why we would want this change, and I must say I'm not very convinced 
>> with the reasoning there.
>>
>> My problem with these changes is that to me it feels like changing the config 
>> file format without some very strong reasons, this will cause all kind of pains 
>> when upgrading from an lm_sensors version with the old format to that with the 
>> new format. Do we really have to do this?
> 
> The two reasons that motivated the change are explained in the ticket,
> there's not so much I can add:
> 
> 1* Removing or adding a module manually is error-prone. Say you have
> the following configuration file:
> 
> MODULE_0=lm90
> MODULE_1=k8temp
> MODULE_2=it87
> 
> Now you change your graphics adapter and the new one no longer has a
> thermal sensor, so you no longer need to load the lm90 driver. Simply
> removing the first line may or may not be enough, depending on how the
> initialization script is implemented. Apparently the one in openSUSE
> copes with that, but the one in our repository (which I think you use
> in Red Hat) doesn't.
> 
> 2* The old format can't be edited with Yast's sysconfig editor, as the
> editor expects fixed variable names. I find it pretty convenient for
> the user to be able to edit all the system settings in a central place,
> with settings put in a function-oriented tree, and a help text for every
> setting. I don't know if Red Hat has an equivalent tool, but if it does
> then I expect it to have the same problem with the current lm-sensors
> configuration file format. It is much easier for me to tell a user to
> open the sysconfig editor and change the value of variable
> HWMON_MODULES to "foo bar" than to tell him/her to
> open /etc/sysconfig/lm_sensors with a text editor, check the highest
> MODULE_%d variable, and add a new line MODULE_%(d+1)ºr.
> 

Hmm, 1 is sort of silly IMHO if you are making changes renumbering really isn't 
that hard to do, 2 to me feels like causing pain for all distros for the 
benefit of one.

Anyways if we are going to make changes to the format, yes we will need a 
migration script and, I think we need to think this through more.

To be more specific in the future we might / will hopefully get support for 
hwmon on graphics cards, then hopefully everything needed will autoload but we 
might need entries in /etc/sysconfig/lm_sensors, so I'm thinking that it would 
be good to have separate lines for things like motherboard sensors, gpu sensors 
and harddisk sensors. This will make it easier for both manual editing as for 
tools for (automatic) configuration.

Regards,

Hans

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

  parent reply	other threads:[~2009-01-13 18:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-12 13:02 [lm-sensors] Updated initialization script, testers wanted Jean Delvare
2009-01-13  8:02 ` Hans de Goede
2009-01-13 11:11 ` Jean Delvare
2009-01-13 18:45 ` Hans de Goede [this message]
2009-01-14 14:28 ` Jean Delvare
2009-01-25  1:28 ` Volker Kuhlmann
2009-01-25 13:47 ` Hans de Goede
2009-01-26 14:24 ` Jean Delvare
2009-02-02 13:27 ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=496CE166.5060803@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=lm-sensors@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.