Bug 451095 - libcxgb3 hardware driver
libcxgb3 hardware driver
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity low
: ---
: ---
Assigned To: Ed Hill
Fedora Extras Quality Assurance
:
Depends On: 450470
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-12 14:59 EDT by Doug Ledford
Modified: 2009-04-10 07:49 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-04-10 07:49:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ed: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Doug Ledford 2008-06-12 14:59:40 EDT
The Chelsio T3 network hardware is iWARP capable 10GigE hardware.  In order to
actually use the iWARP capability from user space, you need the appropriate
hardware driver for use with the libibverbs library.  This package provides that
driver.

Source rpm at:

http://people.redhat.com/dledford/Infiniband/f10/SRPMS

Binary rpms at:

http://people.redhat.com/dledford/Infiniband/f10/x86_64
Comment 1 Dominik 'Rathann' Mierzejewski 2008-06-12 15:25:04 EDT
Providing a specfile separately would make reviewer's life easier. Partial
specfile review below.

> License: GPL/BSD

Invalid License tag. Should be:
License: GPLv2 or BSD

> BuildRequires: libibverbs-devel >= 1.1, autoconf, libtool

libtool requires autoconf

> ExclusiveArch: i386 x86_64 ia64 ppc ppc64

Do you know for certain it doesn't work on sparc/sparc64?

> Obsoletes: %{name}-devel

Why? There's no libcxgb3-devel in Fedora.

> %install
> rm -rf $RPM_BUILD_ROOT
> %makeinstall

Don't use %makeinstall, use %{__make} install DESTDIR=$RPM_BUILD_ROOT instead
(and make sure the Makefile supports DESTDIR for install target).

> install -m 644 -D %{SOURCE1}
${RPM_BUILD_ROOT}%{_sysconfdir}/modprobe.d/libcxgb3.conf

Use install -p to preserve source timestamps.
Comment 2 Doug Ledford 2008-06-12 16:05:53 EDT
(In reply to comment #1)
> Providing a specfile separately would make reviewer's life easier. Partial
> specfile review below.

specfile now available in the SRPM directory.

> > License: GPL/BSD
> 
> Invalid License tag. Should be:
> License: GPLv2 or BSD

Fixed

> > BuildRequires: libibverbs-devel >= 1.1, autoconf, libtool
> 
> libtool requires autoconf

Fixed

> > ExclusiveArch: i386 x86_64 ia64 ppc ppc64
> 
> Do you know for certain it doesn't work on sparc/sparc64?

No, but I also don't know that it *does* work on sparc/sparc64, and this isn't
your run of the mill user space package, it is a hardware driver in user space.
 Regardless though, I removed the ExclusiveArch.

> > Obsoletes: %{name}-devel
> 
> Why? There's no libcxgb3-devel in Fedora.

Because the upstream people distribute a package called libcxgb3-devel that's
really nothing but a .a static lib, and we put that in the -static file.  This
prevents conflicts when upgrading from the libcxgb3 included as part of the
OFED-1.3.tar.gz package by upstream to this package.

> > %install
> > rm -rf $RPM_BUILD_ROOT
> > %makeinstall
> 
> Don't use %makeinstall, use %{__make} install DESTDIR=$RPM_BUILD_ROOT instead
> (and make sure the Makefile supports DESTDIR for install target).

Fixed.

> > install -m 644 -D %{SOURCE1}
> ${RPM_BUILD_ROOT}%{_sysconfdir}/modprobe.d/libcxgb3.conf
> 
> Use install -p to preserve source timestamps.

Fixed.

I updated over the top of the previous package (aka, I built with the same
n-v-r, not a bumped one) so to get the new srpm you might have to force a
refresh if you are behind a proxy cache.  Otherwise, the specfile is now
available and includes the fixes listed.
Comment 3 Ed Hill 2008-06-28 11:53:17 EDT
Here's a quick review:

GOOD:
+ source matches upstream SHA1SUM:
    4aa38c77c42ebbd07d724f2df6cf2e108af03c8d  libcxgb3-1.2.1.tar.gz
    4aa38c77c42ebbd07d724f2df6cf2e108af03c8d  libcxgb3-1.2.1.tar.gz.UP
+ license is correct and correctly included in the main package
+ specfile looks clean and macros sane
+ *.la files are removed
+ builds in mock F8 x86_64
+ rpmlint reports:
    libcxgb3.src:12: W: unversioned-explicit-obsoletes %{name}-devel
    libcxgb3.x86_64: W: non-conffile-in-etc /etc/modprobe.d/libcxgb3.conf
    libcxgb3.x86_64: W: non-conffile-in-etc /etc/libibverbs.d/cxgb3.driver
    libcxgb3.x86_64: W: obsolete-not-provided libcxgb3-devel
    libcxgb3-static.x86_64: W: no-documentation
+ dir ownership looks good
+ permissions look good


I think the "Obsoletes: %{name}-devel" is harmless.  If, as Doug points 
out, it helps users upgrade from upstream/3rd-party RPMs then I'd like 
to allow it.

I don't see any blockers here so its APPROVED.
Comment 4 Doug Ledford 2008-06-28 19:11:31 EDT
New Package CVS Request
=======================
Package Name: libcxgb3
Short Description: Userspace driver for Chelsio iWARP hardware
Owners: dledford
Branches: F-8 F-9
InitialCC:
Cvsextras Commits: yes
Comment 5 Kevin Fenzi 2008-06-29 00:21:27 EDT
cvs done.
Comment 6 Peter Lemenkov 2009-04-10 00:46:14 EDT
What's the status of this ticket? Should we close it?
Comment 7 Doug Ledford 2009-04-10 07:42:23 EDT
Yes, I think so.  It's in CVS now.

Note You need to log in before you can comment on or make changes to this bug.