Bug 223117 - [PATCH] sysutils/nut USB drivers process errors in wrong way and sometimes kill themselves without real reason
Summary: [PATCH] sysutils/nut USB drivers process errors in wrong way and sometimes ki...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Cy Schubert
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2017-10-19 15:28 UTC by Lev A. Serebryakov
Modified: 2017-10-20 07:50 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (cy)


Attachments
Patch to place into file/ directory. (14.30 KB, patch)
2017-10-19 15:28 UTC, Lev A. Serebryakov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Lev A. Serebryakov freebsd_committer freebsd_triage 2017-10-19 15:28:33 UTC
Created attachment 187311 [details]
Patch to place into file/ directory.

It is well-known problem, mostly with blazer_usb drive, but not limited to it, that sometimes USB driver reports "Permission error" and exits without any visible reason (permissions are Ok or driver run as root). This problem was reported on mailing lists and forums for FreeBSD 9, 10 and 11, and not for other operating systems.

I found reason for these problems: current (2.7.4) version of NUT uses libusb-0.1 API. All processing of USB errors in NUT sources assumes that "return values less than zero" of libusb-0.1 calls means "negated errno codes", and have special processing for some errors, like "-EPERM" (which is -1, as EPERM is 1). 

FreeBSD uses wrappers around libusb-1.0 API to provide libusb-0.1 API and uses libusb-1.0 error codes (which are negative!), and -1 is LIBUSB_ERROR_IO, whcih should not kill driver but must trigger reconnect.

NUT has project to use libusb-1.0 API, and, I hope, they fix this problem in the course of this project (I've reported this problem to NUT developers), but for now I've hacked up quick-n-dirty patch to fix problem with nut 2.7.4.

Main problem with this patch, is I copy'n'pasted error definitions from "/usr/include/libusb.h", as I found that it is hard to include system include together with project-local include.

Patch is attached. Please note, that it is patch for "files/" directory, not patch for port directory, PORTREVISION must be tweaked as well.
Comment 1 Cy Schubert freebsd_committer freebsd_triage 2017-10-20 00:57:01 UTC
Looks good.

Applied it to my management machine's nut. No issues. I'll commit it right away. Thank you for the patch.
Comment 2 commit-hook freebsd_committer freebsd_triage 2017-10-20 01:09:36 UTC
A commit references this bug:

Author: cy
Date: Fri Oct 20 01:09:13 UTC 2017
New revision: 452496
URL: https://svnweb.freebsd.org/changeset/ports/452496

Log:
  Nut USB drivers report a "permission error" without visible reasons
  for the error even though permissions are OK (or the driver is run
  as root).

  Nut uses libusb-0.1 API, assuming return cods of < 0. FreeBSD provides
  a libusb-0.1 wrapper howerver it uses libusb-1.0 error codes (which
  are negative). This set of patches "teaches" nut libusb-1.0 error codes
  as produced by FreeBSD.

  Network UPS Tools (networkupstools.org) has a project to use libusb-1.0.
  This commit is a stopgap fix until our upline implments lubusb-1.0
  support in nut.

  PR:		223117
  Submitted by:	lev

Changes:
  head/sysutils/nut/Makefile
  head/sysutils/nut/files/patch-drivers_blazer__usb.c
  head/sysutils/nut/files/patch-drivers_libshut.c
  head/sysutils/nut/files/patch-drivers_libusb.c
  head/sysutils/nut/files/patch-drivers_libusb.h
  head/sysutils/nut/files/patch-drivers_nutdrv__qx.c
  head/sysutils/nut/files/patch-drivers_riello__usb.c
  head/sysutils/nut/files/patch-drivers_tripplite__usb.c
  head/sysutils/nut/files/patch-drivers_usbhid-ups.c
Comment 3 Cy Schubert freebsd_committer freebsd_triage 2017-10-20 05:58:45 UTC
Did you test this under stable?
Comment 4 Alexander Zagrebin 2017-10-20 07:42:19 UTC
Doesn't builds on 11.1-RELEASE:
drivers/libshut.c and drivers/usbhid-ups.c has to include libusb.h for successful build.
Comment 5 Cy Schubert freebsd_committer freebsd_triage 2017-10-20 07:50:46 UTC
Looks like it doesn't build on stable/11 nor on current on i386. We can continue to track this on 223122. Just posted a patch there.