openwrt / packages

Community maintained packages for OpenWrt. Documentation for submitting pull requests is in CONTRIBUTING.md
GNU General Public License v2.0
4.03k stars 3.5k forks source link

nut package #6843

Closed malakingpusa closed 6 years ago

malakingpusa commented 6 years ago

Maintainer: @cshoredaniel Environment: (ar71xx, TP-Link Archer C7 v4, OpenWrt 18.06.0)

Description: Seems there is a race condition which prevents upsd from starting.

Below is a portion of the nut-server script:

if [ "$have_drivers" = "true" ]; then
        $DEBUG /usr/sbin/upsd ${runas:+-u $runas} $OPTIONS
        $DEBUG /usr/sbin/upsdrvctl ${runas:+-u $runas} start
        fi

If I swap the lines and insert a 10-second delay, upsd starts.

if [ "$have_drivers" = "true" ]; then                                                                                                                                       
            $DEBUG /usr/sbin/upsdrvctl ${runas:+-u $runas} start
            sleep 10
            $DEBUG /usr/sbin/upsd ${runas:+-u $runas} $OPTIONS
    fi

Also, when I try to execute forced shutdown command (upsdrvctl shutdown), I get the following error:

root@TPLINKAC1750:~# upsdrvctl shutdown
Network UPS Tools - UPS driver controller 2.7.4
Network UPS Tools - Generic HID driver 0.41 (2.7.4)
USB communication driver 0.33
Can't claim USB device [0764:0501]: No such file or directory
Driver failed to start (exit status=1)

Also, when I try to read the USB descriptors, it says "UNAVAILABLE".

root@TPLINKAC1750:~# lsusb -v -d 0764:0501

Bus 001 Device 003: ID 0764:0501 Cyber Power System, Inc. CP1500 AVR UPS
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x0764 Cyber Power System, Inc.
  idProduct          0x0501 CP1500 AVR UPS
  bcdDevice            0.01
  iManufacturer           3 CPS
  iProduct                1 CP1500EPFCLCD
  iSerial                 2 CRNGV2000037
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           34
    bNumInterfaces          1
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0xc0
      Self Powered
    MaxPower                2mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0 No Subclass
      bInterfaceProtocol      0 None
      iInterface              0 
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.10
          bCountryCode           33 US
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength     480
         Report Descriptors: 
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0040  1x 64 bytes
        bInterval              10
Device Status:     0x0000
  (Bus Powered)

There seems to be a hotplug issue on the USB. If I unplug/re-plug the USB, the USB descriptors are complete when queried and the forced shutdown command succeeds.

danielfdickinson commented 6 years ago

@malakingpusa Thank you for filing this. Since our discussion on the forum (https://forum.openwrt.org/t/network-ups-tools-nut-on-tplink-archer-c7-v4-using-lede-18-06-0/18610 for others), I've spent some time looking at this. I have a fix for the hotplug issue in the works but the USB issue is more difficult. At this point I'm actually thinking the problem is an ar71xx/ath79 kernel-level USB issue. I didn't notice the same problem on a Pi B+, but I will be making a build later in the week to confirm whether that's actually true.

danielfdickinson commented 6 years ago

To be clear, so far, I can reproduce the USB issue on my ar71xx/ath79 board but not the Pi.

malakingpusa commented 6 years ago

Just to confirm, I also have a Raspberry Pi B+ and nut seems to be working perfectly fine even if I have 3 UPSes connected.

danielfdickinson commented 6 years ago

@malakingpusa Thank you for confirming that!. RL is likely to be taking a big chunk out of my ability to work on this for a week or two, so if you could file a bug about this with https://bugs.openwrt.org (I'd start with the descriptors and emphasize that Pi works but not ar71xx so that it doesn't get sloughed off as a 'nut' issue.

The red flag for me is that there are issues like descriptors that have nothing to do with NUT. NUT not working is more a by-product of kernel issues in my view (I could be wrong, but evidence to me leads to not a specifically NUT issue).

malakingpusa commented 6 years ago

Thanks @cshoredaniel . Just added a task in OpenWrt bugs.

danielfdickinson commented 6 years ago

Posted to the task: Looks like a procd complication rather than kernel.

On further investigation the descriptor issue is not a driver issue - it only occurs when the NUT driver is active. Also /lib/nut/usbhid-ups -a upsname -k does shutdown the UPS if the driver is not already running. So it looks like the real issue has to do with NUT playing well with PROCD (that is signals not being handled as usual by nut because of something or procd, or some similar issue). Hence this issue can be closed and further details will be on: https://github.com/openwrt/packages/issues/6843 unless the issue turns out to be unresolvable without procd changes, in which case a procd ticket will be opened.

danielfdickinson commented 6 years ago

Ok, testing upsdrvctl independently of the initscripts shows that the issue with NUT itself. Playing duckduckgo.com for the error messages that show up, it appears that it's either a libusb-compat issue (i.e. the fact NUT is still using ancient libusb 0.1 stuff) or interaction with the init system. Either way I'm going to look the Debian patches as there appears to be some dealing with this for systemd there.

danielfdickinson commented 6 years ago

Ok, testing upsdrvctl independently of the initscripts shows that the issue with NUT itself. Playing duckduckgo.com for the error messages that show up, it appears that it's either a libusb-compat issue (i.e. the fact NUT is still using ancient libusb 0.1 stuff) or interaction with the init system.

danielfdickinson commented 6 years ago

Reported upstream https://github.com/networkupstools/nut/issues/602 and am working on a fix.

danielfdickinson commented 6 years ago

@malakingpusa Some news!

  1. upsdrvctl shutdown is intended to be run once all NUT daemon have stopped, as part of the final forced shutdown, so the docs indicating using it for testing are a little misleading. You can use it for testing, but in order to have a valid config in OpenWrt you need to create the UCI config, start the nut-srever initscript, then stop it...then you can use upsdrvctl shutdown.

  2. I discovered that I missed the shutdown of the UPS in the default configs. The current configs shutdown the system, but don't do UPS forced shutdown via e.g. upsmon -c fsd (I didn't know about that command and I had a small battery UPS that I unplugged for testing, so I missed the FSD scenario). So that is something I'm going to try to fix in an appropriate way for backporting to 18.06 branch.

  3. I have proper PROCD stuff in the works,and I'm hoping to backport as well, because the procd was interacting badly with NUT daemonization, which hadn't been a problem for me when the nut changes were introduced 2-3 years ago, and the previous maintainer seemed to be doing okay with the changes too, so I'm thinking there have been procd behaviour changes (certainly a lot of intervening commits).

malakingpusa commented 6 years ago

Thanks saw your upstream report.

I agree with #1 that upsdrvctl shutdown is for testing. In my NUT on raspberry pi B+, I actually use /lib/nut/usbhid-ups -a upsname -k to do fsd.

By the way, my intended system is to use the Archer C7 router to control multiple UPSes. During detection of a certain low battery level on an onbattery event, I will send the FSD command to other UPSes before sending the FSD to the UPS that powers the router.

Will wait for #2 and #3 updates. Thanks again.

danielfdickinson commented 6 years ago

@malakingpusa Hopefully I get it right, but with what I'm working on really all you may need to do is set the variables on the UPSes for battery level, and things ought to 'Just Work' :-) I'm adding a test to the driver shutdown for the presence of the POWERDOWNFLAG (actually hard-coding /etc/killpower and will document that if one wants something else to edit the nut-driver initscript that is now a proper procd script). I suppose I could configure POWERDOWNFLAG in the nut_driver config file too, but I'm not sure that's really needed (in fact I will de-uci POWERDOWNFLAG and insist on editing nut-monitor initscript too, just to emphasize that you'd better know what you're doing (since not having access to the FS with the flag is kind of bad, and failing to edit the nut-driver script (or if it were in a config, sync the configs) is an easily made critical error.

malakingpusa commented 6 years ago

@cshoredaniel okay will wait for the fix then. Appreciate the effort you put into this so far to help.

danielfdickinson commented 6 years ago

@malakingpusa Not a problem; it's given me the impetus to get this properly procdified, and helped me uncover a few things I missed when I did the original work on updating the package. I'm just surprised there's not been more comments until now.

danielfdickinson commented 6 years ago

@malakingpusa pushed PR #6897 which once Travis behaves ought to solve all the issues. This involves too many changes to backport to 18.06 series I think so I have to figure out what is essential for 18.06 (probably just the nutshutdown script and related changes).

malakingpusa commented 6 years ago

@cshoredaniel Let me check both commits and test. Thanks.

danielfdickinson commented 6 years ago

@malakingpusa Great! It's always good to have a second pair of eyes. Let me know once you've had a chance to test.

danielfdickinson commented 5 years ago

@malakingpusa FYI I've posted PR: https://github.com/openwrt/packages/pull/7638 which ought to do must of what's now in master except the USB hotplug bits (as that's a new feature IMO). If you please test and comment!

malakingpusa commented 5 years ago

What was the USB hotplug bits about?

Regards, Jag

Sent from my mobile device

On 11 Dec 2018, at 20:36, Daniel Dickinson notifications@github.com wrote:

@malakingpusa FYI I've posted PR: #7638 which ought to do must of what's now in master except the USB hotplug bits (as that's a new feature IMO). If you please test and comment!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

danielfdickinson commented 5 years ago

On 2018-12-11 8:35 p.m., Jag Glifonea wrote:

What was the USB hotplug bits about?

When a USB hotplug event creates a new USB device, check if it's a UPS and if so make the ownership and permissions so that one can use 'option runas nut' for the driver and have it 'just work'. Without it, it's easier to have the driver and upsd etc running as root (which is the behaviour from 17.x), although one could theoretically write one's own hotplug script that sets the perms on USB hotplug for one's specific device.

Regards,

Daniel

malakingpusa commented 5 years ago

Thanks Daniel.

Regards,

Jag

From: Daniel Dickinson notifications@github.com Sent: Wednesday, 12 December 2018 10:28 To: openwrt/packages packages@noreply.github.com Cc: Jag Glifonea jag@xeptocomputing.com; Mention < mention@noreply.github.com> Subject: Re: [openwrt/packages] nut package (#6843)

On 2018-12-11 8:35 p.m., Jag Glifonea wrote:

What was the USB hotplug bits about?

When a USB hotplug event creates a new USB device, check if it's a UPS and if so make the ownership and permissions so that one can use 'option runas nut' for the driver and have it 'just work'. Without it, it's easier to have the driver and upsd etc running as root (which is the behaviour from 17.x), although one could theoretically write one's own hotplug script that sets the perms on USB hotplug for one's specific device.

Regards,

Daniel

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/openwrt/packages/issues/6843#issuecomment-446438339, or mute the thread https://github.com/notifications/unsubscribe-auth/AooEg6FyafbU6cDvsKR20xLNeuKaF1Vsks5u4GmzgaJpZM4WHEZU .