[Bug 1404043] Review Request: rdma-core - RDMA core userspace libraries and daemons

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



https://bugzilla.redhat.com/show_bug.cgi?id=1404043



--- Comment #5 from Jarod Wilson <jarod@xxxxxxxxxx> ---
(In reply to Jarod Wilson from comment #4)
> Spec file comments from honli:
> 
> Url: https://github.com/linux-rdma/rdma-core
> honli:
> https://fedoraproject.org/wiki/Packaging:
> SourceURL#When_Upstream_uses_Prohibited_Code
> honli: Please add comment for the "Source:" tag.
> Source: rdma-core-%{version}.tgz

I could, but this situation is temporary, there will be an upstream release and
a full URL shortly. This was noted in comment #1, but I could add that same
text to the spec until the release is out if need be.

> BuildRequires: binutils
> BuildRequires: cmake >= 2.8.11
> BuildRequires: gcc
> BuildRequires: libudev-devel
> BuildRequires: pkgconfig
> BuildRequires: pkgconfig(libnl-3.0)
> BuildRequires: pkgconfig(libnl-route-3.0)
> BuildRequires: valgrind-devel
> 
> honli: libnl3-devel is required for iwpmd libibverbs

That's what you get from pkgconfig(libnl-3.0).

> honli: systemd and dracut also needed.

These already get pulled in.

> honli: You also delete a few necessary "Requires:" tags.
> honli:  Please see review-comment.txt for details.

>From that doc:

> honli: Please add "Requires:" entries against rsyslog, systemd, kmod,
> honli: logrotate, initscripts, and dracut.

None of these need to be listed explicitly, so far as I know. These are all
things that are expected to be on the system, thus no need to call them out
specifically. Several of them are already requirements of the kernel.


> %description
> RDMA core userspace infrastructure and documentation.
> honli: I know this is an upstream issue. This is %description
> honli: section is too simple, in other words, it is meaningless.
> honli: If you do not have good %description section, I suggest
> honli: you copy and paste
> https://bugzilla.redhat.com/show_bug.cgi?id=1404043#c0
> honli: At least, user will know what is the package after read that.

That #c0 text applies to the package as a whole though, including things that
are in libibverbs. I'll expand on the text there a bit though.

Proposed update:

%description
RDMA core userspace infrastructure and documentation, including initscripts,
kernel driver-specific modprobe override configs, IPoIB network scripts,
dracut rules, and the rdma-ndd utility.

> %package -n libibverbs
> Summary: A library and drivers for direct userspace use of RDMA
> (InfiniBand/iWARP) hardware
> Requires(post): /sbin/ldconfig
> Requires(postun): /sbin/ldconfig
> Requires: rdma-core
> honli: Even over 99% files are stext files, rdma-core contains
> /usr/sbin/rdma-ndd,
> honli: which is an ELF file, so rdma-core is not a noarch rpm. Please
> replace all
> honli: "Requires: rdma-core" with "Requires: %{name}%{?_isa} =
> %{version}-%{release}"

Will do.

> %description -n srp_daemon
> In conjunction with the kernel ib_srp driver, srptools allows you to
> honli: should replace srptools with srp_daemon in above line?
> discover and use SCSI devices via the SCSI RDMA Protocol over InfiniBand.

Done.

> %prep
> %setup
> honli: %setup -q ???
> %build

Why? I prefer verbose myself, so you can see more easily in logs if something
goes wrong.

> %postun -n ibacm
> %systemd_postun_with_restart ibacm.service
> 
> honli: Why you only run
> systemd_post/systemd_preun/systemd_postun_with_restart
> honli: against ibacm.service? How about iwpmd.service and srp_daemon.service?

Looks like an oversight, will correct this.

> %post -n libibcm -p /sbin/ldconfig
> %postun -n libibcm -p /sbin/ldconfig
> honli: Why not run ldconfig for other libraries, at least librdmacm has .so
> honli: file in private dir.

Another oversight. And on a related note, the ldconfigs for the base package
serve no purpose, since there are no libs in it. This is probably a remnant of
starting from the one-shot spec in the top level of the source dir. Will fix
this up too.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]