Bug 12831 - vnstat seems to change it cmd options
Summary: vnstat seems to change it cmd options
Status: CLOSED FIXED
Alias: None
Product: IPFire
Classification: Unclassified
Component: --- (show other bugs)
Version: 2
Hardware: unspecified Unspecified
: - Unknown - Minor Usability
Assignee: Matthias Fischer
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-04-02 08:02 UTC by Jonatan Schlag
Modified: 2022-06-13 14:26 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Jonatan Schlag 2022-04-02 08:02:51 UTC
Hi,

while working on our network (see https://git.ipfire.org/?p=people/jschlag/ipfire-2.x.git;a=shortlog;h=refs/heads/improve_network_startup) I found out that the command line  options of vnstat have changed.:

 /usr/bin/vnstat -u -i ${DEVICE} -r --enable --force > /dev/null 2>&1

complains about:

-u
-r
--enable

This command line gives no error but I am not an expert if it do what it is supposed to do:

 /usr/bin/vnstat -i ${DEVICE} --force > /dev/null 2>&1

This seems to be broken a long time. The -u switch was removed in version 2.0 (https://humdi.net/vnstat/CHANGES) which we introduced nearly two years ago https://git.ipfire.org/?p=ipfire-2.x.git;a=commit;h=7b1134ea606428f05f721a7a598b99556f39d8dc

As I am not an expert in vnstat I see myself unable to fix this.
Comment 1 Michael Tremer 2022-04-04 09:48:30 UTC
@Matthias: Is this one for you?
Comment 2 Adolf Belka 2022-04-04 10:34:37 UTC
I have had a look through the changelogs on this and the only one mentioned is --enable being removed in 2018. Nothing replacing it.

--enable, -r --reset and -u --update are all no longer in the vnstat man file.

Looking through an older version I found the following

--enable is not needed when the daemon is running.

--update is not supported when the daemon is running.

--reset is not needed when the daemon is used.

Looking through IPFire we do start and run vnstad which is the daemon. This would suggest that those three options were never doing much anyway.
Comment 3 Michael Tremer 2022-04-04 15:17:44 UTC
Thank you, Adolf. Very interesting.

Who wants to remove them from our scripts then?
Comment 4 Matthias Fischer 2022-04-04 16:32:57 UTC
I must confess, I totally overlooked that part of 'vnstat' - sorry for the mess.

However, since that new version ran absolutely smoothly in my tests, I didn't investigate further at the time.

If these changes just mean that removing the offending 'vnstat'-commands from '/etc/rc.d/init.d/green, red, orange, blue' would be sufficient, then I would want to take a chance.

Otherwise I'd recommend that one of 'experts' does this - I don't want to mess things more up than absolutely necessary...
Comment 5 Adolf Belka 2022-04-04 17:57:39 UTC
(In reply to Matthias Fischer from comment #4)
> I must confess, I totally overlooked that part of 'vnstat' - sorry for the
> mess.

You would have had trouble seeing it. You updated from 2.6 to 2.7 but the removal looks to have happened in version 2.0 in 2018, at least for the removal of --uppdate and the other two option removals are not mentioned in the changelog at all.
> 
> However, since that new version ran absolutely smoothly in my tests, I
> didn't investigate further at the time.
> 
> If these changes just mean that removing the offending 'vnstat'-commands
> from '/etc/rc.d/init.d/green, red, orange, blue' would be sufficient, then I
> would want to take a chance.
> 
> Otherwise I'd recommend that one of 'experts' does this - I don't want to
> mess things more up than absolutely necessary...

The options have not been available in IPFire since April 2020, so around two years, and the old man page indicates that none of those options are used when the vnstat daemon is running.

Certainly removing them would not change anything compared to the last two years.
Comment 6 Peter Müller 2022-04-10 09:54:58 UTC
https://git.ipfire.org/?p=people/pmueller/ipfire-2.x.git;a=commit;h=5806ff0cc5af4b361b3e32cb9e32d97d1f07d400

Not bumping to MODIFIED since this is my personal temporary branch for Core Update 168, but I expect this change to land in "next" soon.
Comment 7 Matthias Fischer 2022-04-10 10:26:32 UTC
@Peter:
Thanks for adding the GIT link...tired head: I forgot that ...

@Michael:
Since there was no any further comment on the list: is the explanation you asked for "why these command are not needed" sufficient? I found no better way.
Comment 8 Peter Müller 2022-04-10 11:14:09 UTC
> Thanks for adding the GIT link...tired head: I forgot that ...

Absolutely no worries about it. I do not consider that your task anyway,
since you cannot know the precise link until your patch has been commited.
It is more efficient if the person who does amend patches just updates
the corresponding Bugzilla entry.

Have some rest (its Sunday, after all) and do not worry about that. :-)

All the best,
Peter Müller