On 2017-04-12 12:24, Paul Wise wrote: > This was an unfortunate outcome for those of us who use sensord but do > not use the RRD mode at all. I think it would have been better to just > disable the RRD mode, but that wouldn't have been the right solution.
What's your use case for sensord without RRD? Logging values and alarms into syslog? In that case we might just want to remove/disable RRD support while keeping the rest of the features. > It seems that all of lm-sensors is dead upstream, not just sensord. This is true, that said while lm-sensors is dead for a bit less than 2 years now, there hasn't been any change to sensord for more than 5 years, and no major change for more than 8 years. > On Wed, 2017-04-12 at 10:31 +0800, Paul Wise wrote: > > > I've attached a patch which renames the old RRD file > > and creates a new one. Could we add it and restore sensord? > Attached a better version of the patch with two fixes: > > * No memory leak > * Updates the new RRD after creation > > -- > bye, > pabs > > https://wiki.debian.org/PaulWise > --- a/prog/sensord/rrd.c > +++ b/prog/sensord/rrd.c > @@ -456,8 +456,34 @@ > "sensord", sensord_args.rrdFile, rrdBuff, NULL > }; > if ((ret = rrd_update(3, (char **) /* WEAK */ argv))) { > + /* Cope with incompatible RRD files by creating new > + ones and saving a backup of the old RRD files. */ > + const char *format = "%s.old"; > + size_t len = strlen(format) - 2 + > strlen(sensord_args.rrdFile) + 1; > + char *backup = (char*)malloc(len); Hmm, it only works for one change, after that the .old file will be overwritten causing the data loss. I guess the filename should include the date and time. > sensorLog(LOG_ERR, "Error updating RRD file: %s: %s", > sensord_args.rrdFile, rrd_get_error()); > + if (backup == NULL) > + goto cleanup; > + if (snprintf(backup, len, format, sensord_args.rrdFile) > >= len) > + goto cleanup; > + if (rename(sensord_args.rrdFile, backup) != 0) > + goto cleanup; > + sensorLog(LOG_ERR, "Incompatible RRD file renamed from > %s to %s", > + sensord_args.rrdFile, backup); > + ret = rrdInit(); > + if (!ret) { > + sensorLog(LOG_ERR, "New RRD file created at %s", > + sensord_args.rrdFile); > + ret = rrd_update(3, (char **) /* WEAK */ argv); > + } else { > + rename(backup, sensord_args.rrdFile); > + } > + cleanup: > + if (backup != NULL) > + free(backup); > + if (ret) > + return ret; > } > } > sensorLog(LOG_DEBUG, "sensor rrd updated"); While that might work (provided we keep multiple versions of the file), I think the data are not really usable as each period will have a different format. The real way to fix that would be to use one RRD file per sensor, but it becomes a major change to the software, and I am not sure we want to do that given the dead upstream Cheers, Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
signature.asc
Description: PGP signature
