Bug 462311 - Review Request: raidutils - Utilities to manage Adaptec I2O compliant RAID controllers
Review Request: raidutils - Utilities to manage Adaptec I2O compliant RAID co...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-09-15 06:18 EDT by Dominik 'Rathann' Mierzejewski
Modified: 2009-04-14 11:34 EDT (History)
4 users (show)

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


Attachments (Terms of Use)

  None (edit)
Description Dominik 'Rathann' Mierzejewski 2008-09-15 06:18:29 EDT
Spec URL: http://rpm.greysector.net/fedora/raidutils.spec
SRPM URL: http://rpm.greysector.net/fedora/raidutils-0.0.6-1.fc9.src.rpm
Description:
The raidutils program allow the user to manage the Adaptec I2O compliant RAID
controllers. It can, for example, create/delete an RAID array, add/remove a hot
spare drive to/from a RAID array, activate/silence the alarm or get information
about the status of the RAID array and disks.
Comment 1 Jaroslaw Gorny 2008-12-23 17:04:31 EST
I've reviewed requested package although I'm not a member of 'packager' group. It builds correctly and does all the items market as 'MUST' on Review Guidelines.
Comment 2 Dominik 'Rathann' Mierzejewski 2009-01-08 15:27:35 EST
Spec URL: http://rathann.fedorapeople.org/review/raidutils.spec
SRPM URL: http://rathann.fedorapeople.org/review/raidutils-0.0.6-2.fc9.src.rpm

Slightly updated to auto-load the necessary module on boot (the tool won't work without /dev/i2octl present).
Comment 3 Nicolas Chauvet (kwizart) 2009-01-26 12:15:48 EST
This software is from 2005. Why do you need you still need this ?
Why Fedora tools aren't enought for Adaptec support. (dmraid should support most hardware raid controller nowadays)
 
Why is there patches not upstreamed in a newer release ?

This package deserve a "-"  (not maintained from an upstream)
Comment 4 Dominik 'Rathann' Mierzejewski 2009-01-26 12:27:33 EST
(In reply to comment #3)
> This software is from 2005. Why do you need you still need this ?
> Why Fedora tools aren't enought for Adaptec support. (dmraid should support
> most hardware raid controller nowadays)

I need this for an old Adaptec (formerly DPT) hardware RAID controller. Frankly I haven't tried using dmraid to access it. I'll investigate that.

> Why is there patches not upstreamed in a newer release ?

The patch from Debian is necessary to build this on recent Fedora.

> This package deserve a "-"  (not maintained from an upstream)

There are many packages in Fedora where upstream is not active.
Comment 5 Dominik 'Rathann' Mierzejewski 2009-01-27 05:59:27 EST
(In reply to comment #4)
> (In reply to comment #3)
> > This software is from 2005. Why do you need you still need this ?
> > Why Fedora tools aren't enought for Adaptec support. (dmraid should support
> > most hardware raid controller nowadays)
> 
> I need this for an old Adaptec (formerly DPT) hardware RAID controller. Frankly
> I haven't tried using dmraid to access it. I'll investigate that.

OK, I've checked dmraid and it seems to support only various fakeRAID "controllers". Adaptec/DPT 2400A is a real hardware RAID and raidutils are necessary to monitor and manage it without rebooting and entering controller BIOS setup.
Comment 6 Jason Tibbitts 2009-03-08 19:11:32 EDT
rpmlint says:
  raidutils.x86_64: W: undefined-non-weak-symbol /usr/lib64/libraidutil.so.0.0.0 
   Argv
  raidutils.x86_64: W: undefined-non-weak-symbol /usr/lib64/libraidutil.so.0.0.0 
   osdSwap2
  raidutils.x86_64: W: undefined-non-weak-symbol /usr/lib64/libraidutil.so.0.0.0 
   osdSwap4
I guess the executables are expected to provide these.  Since this isn't a library you'd expect to be used by other problems, I don't see a problem here.

  raidutils.x86_64: W: unused-direct-shlib-dependency 
   /usr/lib64/libraidutil.so.0.0.0 /lib64/libm.so.6
This is a minor artifact of autoconf; you can fix it if you like with a quick call to sed but it's probably not worth it.

I do wish the package had a somewhat less generic name, but it's been around for over a decade and I don't see any point in trying to change it now.

I don't see any problems with the upstream being inactive; there's little or no security exposure here, the hardware is no longer sold and the software works.  At least, I'm taking your word that it does; I don't have the hardware.  

There's no reason for BuildRequires: gcc-c++; it's in the default buildroot.  That's really a minor issue; you can take it out when you import the package.

* source files match upstream.  sha256sum:
   ac350f60b9635d952a7a5664effa59e418ada9ad3deba66d46e6e0a094966d65  
   raidutils-0.0.6.tar.bz2
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
X BuildRequires (gcc-c++ unneeded).
* compiler flags are appropriate.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* debuginfo package looks complete.
* rpmlint has acceptable complaints.
* final provides and requires are sane:
   libraidutil.so.0()(64bit)
   raidutils = 0.0.6-2.fc11
   raidutils(x86-64) = 0.0.6-2.fc11
  =
   /bin/sh
   /sbin/ldconfig
   libgcc_s.so.1()(64bit)
   libgcc_s.so.1(GCC_3.0)(64bit)
   libraidutil.so.0()(64bit)
   libstdc++.so.6()(64bit)
   libstdc++.so.6(CXXABI_1.3)(64bit)
   libstdc++.so.6(GLIBCXX_3.4)(64bit)

* shared libraries are installed; ldconfig called properly.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no static libraries.
* no libtool .la files.

APPROVED; just remove the errant gcc-c++ build dependency when you check in.

The package review process needs reviewers!  If you haven't done any package
reviews recently, please consider doing one.
Comment 7 Dominik 'Rathann' Mierzejewski 2009-03-22 20:25:05 EDT
New Package CVS Request
=======================
Package Name: raidutils
Short Description: Utilities to manage Adaptec I2O compliant RAID controllers
Owners: rathann
Branches: EL-5
Comment 8 Dominik 'Rathann' Mierzejewski 2009-03-22 20:26:42 EDT
Thanks for the review. I've just reviewed libass (bug 491550). I'll fix the redundant builddep and look at the weak symbols issue too.
Comment 9 Kevin Fenzi 2009-03-24 13:32:54 EDT
cvs done.
Comment 10 Nicolas Chauvet (kwizart) 2009-04-14 10:45:47 EDT
can be closed or ...?
Comment 11 Dominik 'Rathann' Mierzejewski 2009-04-14 11:34:08 EDT
Indeed.

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