Bug 254048 - Review Request: biosdevname - udev helper for naming devices per BIOS names
Review Request: biosdevname - udev helper for naming devices per BIOS names
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Matthias Saou
Fedora Extras Quality Assurance
:
Depends On:
Blocks: F-ExcludeArch-ppc FE-ExcludeArch-ppc64
  Show dependency treegraph
 
Reported: 2007-08-23 16:04 EDT by Matt Domsch
Modified: 2012-02-09 06:28 EST (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-09-21 14:39:17 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
matthias: fedora‑review+


Attachments (Terms of Use)
proposed patch to the specfile (1.67 KB, patch)
2007-08-24 02:52 EDT, Harald Hoyer
no flags Details | Diff
revised patch (1.69 KB, patch)
2007-08-24 02:56 EDT, Harald Hoyer
no flags Details | Diff

  None (edit)
Description Matt Domsch 2007-08-23 16:04:07 EDT
Spec URL: http://linux.dell.com/biosdevname/biosdevname.spec
SRPM URL: http://linux.dell.com/biosdevname/biosdevname-0.2.1-1.fc7.src.rpm
Description:
biosdevname in its simplest form takes an kernel name name as an
argument, and returns the BIOS-given name it "should" be.  This is necessary
on systems where the BIOS name for a given device (e.g. the label on
the chassis is "Gb1") doesn't map directly and obviously to the kernel
name (e.g. eth0).
Comment 1 Matt Domsch 2007-08-23 22:23:08 EDT
I'm going to ExclusiveArch %{ix86} x86_64 ia64 and put a comment in the spec.

# SMBIOS and PCI IRQ Routing Tables only exist on these arches.  It's
# also likely that other arches don't expect the PCI bus to be sorted
# breadth-first, or of so, there haven't been any comments about that
# on LKML.

Marking this as blocking ppc and ppc64 ExcludeArch trackers.
Comment 2 Harald Hoyer 2007-08-24 02:52:21 EDT
Created attachment 172394 [details]
proposed patch to the specfile

my patch to let it build and install
Comment 3 Harald Hoyer 2007-08-24 02:56:50 EDT
Created attachment 172396 [details]
revised patch
Comment 4 Matthias Saou 2007-08-24 04:53:20 EDT
I just saw the announcement on the poweredge-list. This looks neat! I'll start
the review of this package.
Comment 5 Matthias Saou 2007-08-24 05:01:08 EDT
One immediate question : The resulting binary package depends on the shared
libsysfs.so.2 which is in /usr/lib*/ and not /lib*/ so for Harald's patch needs
further changes to get the package to be able to do anything useful before /usr
is mounted.
Comment 6 Harald Hoyer 2007-08-24 05:13:05 EDT
right... either move libsysfs or make biosdevname static...
Comment 7 Matthias Saou 2007-08-24 05:20:45 EDT
(In reply to comment #6)
> right... either move libsysfs or make biosdevname static...

Yup. Current libsysfs-devel still provides the static library, so that would be
an option, but surely moving libsysfs would be cleaner. From the spec file
comment, it seems like the static libsysfs should already have been used, but it
hasn't been. That comment kind of confuses me overall, though :

# some distros won't have a static lib version of libsysfs, which
# would be needed to generate these .so files

I guess the comment is (at least partially) wrong, since even with the static
libsysfs available, the shared one is used.
Comment 8 Kay Sievers 2007-08-24 06:02:55 EDT
Matt, please just get rid of libsysfs usage. It's an outdated, unmaintained and
useless library.
Comment 9 Matt Domsch 2007-08-24 07:45:09 EDT
OK, I'll nuke the libsysfs usage.  That comes from my use of code from the 
pcmciautils-014 package, but for what is used there, it's easy enough to 
replace with standard C lib calls.

The code can already build a static binary, called biosdevnameS.  I just 
disabled this for now, but if we really need this called before /usr is 
mounted (e.g. inside an initrd or NFS-root or something) it's easy enough to 
build again.
Comment 10 Matt Domsch 2007-08-24 14:06:54 EDT
I've applied Harald's RPM spec patch moving this into /lib* and /sbin.  I
removed the libsysfs dependency completely.  I am building, but not RPM
installing, the static copy of the app, in case we decide we need it later.

This builds and runs on Fedora 7 and OpenSUSE 10.3 beta.  rpmlint results are
empty.  The SuSE and Fedora spec files are identical now, and hopefully they can
stay that way and I can nuke one.

Version 0.2.2 is posted now to http://linux.dell.com/files/biosdevname and
pushed into the git tree.
Comment 11 Matthias Saou 2007-08-27 04:45:28 EDT
Looking better. Two more questions :
- What is the point of creating a shared library which is used only by the
binary from this package? Since the .so symlink is wiped out, nothing will be
able to link against it, so not using the static version of the binary only
makes us loose the time it takes to run ldconfig, right?
- The dynamic file list confuses me. Some distros would have something else than
the .so.0 and .so.0.0.0 files without changing the spec file? (btw, the comment
regarding libsysfs above it needs updating now ;-))
Comment 12 Matt Domsch 2007-08-27 07:38:17 EDT
it's purely a "convenience" library, meaning I want to separate all the table
parsing and lookups from the "frontend" command line application and it's
parsing.  I don't at this point expect other applications to use the
"convenience" library, but at some point in the future, if I clean up the API
and there's good reason something else needs those functions, it can be done.
Comment 13 Matt Domsch 2007-08-27 08:08:08 EDT
The reason for the dynamic file list is that some versions of some distros don't
compile libpci.a with -fPIC, so it can't then be included in a shared library. 
There's autoconf code to detect this and automatically sets enable_shared=no
which results in no libbiosdevname.so.* being created.  (OpenSUSE and SLES are
this way right now, as were earlier versions of Fedora).  This spec builds
Fedora, RHEL, OpenSuSE, and SLES at this point, the dynamic file list being the
only magic involved (to add, or not, depending on existance of those .so.*
files, appropriately).

I'm not tied to the idea of the convenience library.  If it's really that ugly,
it can disappear too.

And yes I can fix the spec file comments.
Comment 14 Matt Domsch 2007-08-27 17:26:08 EDT
I decided to nuke the libbiosdevname.* stuff in upstream, it isn't really 
needed and only causes confusion.  Updated SRPM and spec posted.
Comment 15 Matthias Saou 2007-08-30 11:37:11 EDT
Spec and resulting package look good to go. I'll just need to runtime test the
actual binary package.

The %description could maybe be changed from :

"takes an kernel name"

to

"takes a device kernel name" (or "takes a kernel device name"?)
Comment 16 Matt Domsch 2007-08-30 12:05:35 EDT
Spec %description changed to say "takes a kernel device name".
Comment 17 Matthias Saou 2007-08-30 12:39:10 EDT
Okay, package approved! Seems like my workstation's BIOS doesn't override much,
though (Precision 490), but the package didn't break anything either ;-)
Comment 18 Matthias Saou 2007-09-12 07:13:07 EDT
Ping? Matt, still around? :-)
Please go ahead and request CVS branches creation, import, build, and close ;-)
Comment 19 Matt Domsch 2007-09-17 10:00:54 EDT
I haven't forgotten about this, but I've got a showstopper problem before I am 
comfortable doing a build.  I spoke with Kay about this last week, and he 
wanted an enhancement to udev-114 to let biosdevname suggest the new name 
without overriding the values in 70-persistent-net-names.  I haven't made the 
enhancement to udev yet, so that's what's blocking me on this...  Hopefully 
this week I can spend some time on it.
Comment 20 Matt Domsch 2007-09-21 12:13:25 EDT
New Package CVS Request
=======================
Package Name: biosdevname
Short Description: Udev helper for naming devices per BIOS names
Owners: mdomsch,mebrown
Branches: FC-6 F-7 EL-4 EL-5
InitialCC: harald
Cvsextras Commits: yes
Comment 21 Kevin Fenzi 2007-09-21 12:26:56 EDT
cvs done.
Comment 22 Matt Domsch 2007-09-21 14:39:17 EDT
http://koji.fedoraproject.org/koji/taskinfo?taskID=169642 built successfully.
Closing.

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