Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.

Bug 169744

Summary: Review Request: libmthca - Mellanox hardware support for libibverbs
Product: [Fedora] Fedora Reporter: Roland Dreier <rolandd>
Component: Package ReviewAssignee: Ed Hill <ed>
Status: CLOSED NEXTRELEASE QA Contact: David Lawrence <dkl>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-extras-list
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
URL: http://www.digitalvampire.org/fedora/
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2005-11-08 22:06:33 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 169743    
Bug Blocks: 163779    

Description Roland Dreier 2005-10-02 21:36:17 UTC
Spec Name or Url: http://www.digitalvampire.org/fedora/libmthca.spec
SRPM Name or Url: 
http://www.digitalvampire.org/fedora/libmthca-1.0-0.1.rc3.src.rpm
Description:
libmthca provides a device-specific userspace driver for Mellanox HCAs
(MT23108 InfiniHost and MT25208 InfiniHost III Ex) for use with the
libibverbs library.

I just posted a review request for my libibverbs package in bug 169743.

Comment 1 Ed Hill 2005-10-25 02:43:58 UTC
I'd like to review this package but I don't have any Mellanox IB hardware to 
test it on.  So unless someone says otherwise, I'm going to assume that its OK 
to review based upon all of the criteria *except* actually running/using it.

Comment 2 Michael Schwendt 2005-10-25 09:37:59 UTC
That is okay, assuming that the package maintainer has ways to test
this software release and subsequent releases, too.


Comment 3 Roland Dreier 2005-10-26 04:03:27 UTC
Yes, I have lots of InfiniBand hardware to test with.  I am also the upstream
maintainer of this package.

Comment 4 Roland Dreier 2005-10-27 18:04:38 UTC
Just so I'm clear: should I consider the libmthca package approved?  Or am I
still waiting for Ed's real review?


Comment 5 Ed Hill 2005-10-27 18:26:08 UTC
Hi Roland, per the guidelines:
  http://fedoraproject.org/wiki/PackageReviewGuidelines
I'll do a review and send an approval (or not) as soon as I have some free 
time.  Probably early next week.  Please be patient, I'm just a volunteer 
who has a busy day job, etc.  ;-)

Comment 6 Roland Dreier 2005-10-28 19:23:18 UTC
No worries and no rush -- I just wanted to make sure that I wasn't sitting here
waiting for approval that you had already given.

Comment 7 Roland Dreier 2005-10-30 16:35:27 UTC
New Spec: http://www.digitalvampire.org/fedora/libmthca.spec
New SRPM: http://www.digitalvampire.org/fedora/libmthca-1.0-0.2.rc4.src.rpm

* Wed Oct  5 2005 Roland Dreier <roland> - 1.0-0.2.rc4
- Update to upstream 1.0-rc4 release


Comment 8 Ed Hill 2005-11-03 21:02:41 UTC
Hi Roland, heres a quick review:

perhaps these two need work or perhaps [more likely? ;-)] I just don't 
understand:
 - The devel package includes a static library but there are no header 
     files -- I assume thats because this is a "plug-in library" for 
     libibverbs and it uses the libibverbs-devel headers so it doesn't 
     actually have to provide any headers itself, right?  If so, thats 
     fine but then you should probably have libmthca-devel Require: 
     the libibverbs-devel package
 - a shared library is installed but the usual post/postun ldconfig 
     scripts are not run -- is that really OK?

good:
 + source matches upstream using:
     http://www.digitalvampire.org/fedora/libmthca-1.0-0.2.rc4.src.rpm
 + spec is simple, clean, and readable
 + license is OK and correctly included
 + builds in mock on FC4
 + *.la files correctly removed
 + no errors or warnings from rpmlint

And if someone donates a few compatible IB host adapters and an IB switch, 
I'll gladly test this package on a few cluster nodes running Fedora.  ;-)

Comment 9 Roland Dreier 2005-11-03 21:23:59 UTC
> - The devel package includes a static library but there are no header 
>     files -- I assume thats because this is a "plug-in library" for 
>     libibverbs and it uses the libibverbs-devel headers so it doesn't 
>     actually have to provide any headers itself, right?  If so, thats 
>     fine but then you should probably have libmthca-devel Require: 
>     the libibverbs-devel package

Right, there are no header files because nothing except libibverbs
calls functions in libibverbs.  The static library is just there because
some people have found it useful to link everything staticly into their
app (libibverbs finds static libmthca via dlopen(NULL, )).

You're probably right that libmthca-devel needs to Require: libibverbs-devel,
since libmthca does call libibverbs functions.

> - a shared library is installed but the usual post/postun ldconfig 
>     scripts are not run -- is that really OK?

Do you mean /usr/lib/infiniband/mthca.so?  Again, that's a "plug-in"
that's dlopen()ed by libibverbs, so nothing directly links to it.  So
I don't think we need to do any ldconfig stuff, right?


Comment 10 Ed Hill 2005-11-03 23:44:01 UTC
Hi Roland, that makes sense and tanks for the explanation!

APPROVED.