Spec URL: https://raw.githubusercontent.com/igor-ivanov/libvma/pr/fedora-package/fedora/libvma.spec SRPM URL: https://raw.githubusercontent.com/igor-ivanov/libvma/pr/fedora-package/fedora/libvma-9.0.2-1.src.rpm Description: A library for boosting TCP and UDP traffic (over RDMA hardware) Fedora Account System Username: iivanov This is a re-review request to revive a retired package. koji scratch build for rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=43603199 fc31: https://koji.fedoraproject.org/koji/taskinfo?taskID=43602614 fc32: https://koji.fedoraproject.org/koji/taskinfo?taskID=43602829 fc33: https://koji.fedoraproject.org/koji/taskinfo?taskID=43602945
>Group: System Environment/Libraries This tag is not used in Fedora and not in EPEL. Please remove those instead of hiding them behind %if. >BuildRoot: %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX) >Prefix: %{_prefix} >... >%install >[ "${RPM_BUILD_ROOT}" != "/" -a -d ${RPM_BUILD_ROOT} ] && rm -rf ${RPM_BUILD_ROOT} >... >%clean The tags are not used in Fedora; the buildroot should not be deleted at start of %install. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_tags_and_sections >ExclusiveArch: x86_64 ppc64le ppc64 aarch64 Add a comment explaining why this is needed. >%files >%{_libdir}/%{name}.so The un-versioned .so typically goes into -devel. https://docs.fedoraproject.org/en-US/packaging-guidelines/#_devel_packages
Thank you for review. Spec is updated.
I'm taking over this bug after exchanged email with Artur Iwicki. libvma was introduced in Fedora with bz1298665 . It was retired because not fix FTBFS bz1736055 in time.
1 %{!?configure_options: %global configure_options %{nil}} 2 %{!?use_extralib: %global use_extralib 0} 3 4 %global pmake %{__make} %{?_smp_mflags} %{?mflags} V=1 %make_build 5 %global use_systemd %(if ( test -d "%{_unitdir}" > /dev/null); then echo -n '1'; else echo -n '0'; fi) always be true 6 7 Name: libvma 8 Version: 9.0.2 9 Release: 1%{?dist} 10 Summary: A library for boosting TCP and UDP traffic (over RDMA hardware) 11 12 License: GPLv2 or BSD 13 Url: https://github.com/Mellanox/%{name} Please use full path https://github.com/Mellanox/libvma 14 Source0: %{url}/archive/%{version}/%{name}-%{version}.tar.gz Again. Use https://github.com/Mellanox/libvma/archive/%{version}/%{name}-%{version}.tar.gz Fedora upstream mointor tool will automaticlly try to build new package when new release available in upstream. An working source URL is necessary. 15 16 # libvma currently supports only the following architectures 17 ExclusiveArch: x86_64 ppc64le ppc64 aarch64 18 19 BuildRequires: pkgconfig 20 BuildRequires: automake 21 BuildRequires: autoconf 22 BuildRequires: libtool 23 BuildRequires: gcc-c++ 24 BuildRequires: libibverbs-devel 25 BuildRequires: librdmacm-devel 26 %if 0%{?rhel} >= 7 || 0%{?fedora} >= 24 || 0%{?suse_version} >= 1500 line 26 is always true for Fedora and RHEL release in today. Please remove anything related to suse for Fedora spec file. So, please delete line 26 and line 28. 27 BuildRequires: libnl3-devel 28 %endif 29 30 %description 31 libvma is a LD_PRELOAD-able library that boosts performance of TCP and 32 UDP traffic. It allows application written over standard socket API to 33 handle fast path data traffic from user space over Ethernet and/or 34 Infiniband with full network stack bypass and get better throughput, 35 latency and packets/sec rate. 36 . Unwanted dot in line 36, please delete it. 37 No application binary change is required for that. 38 libvma is supported by RDMA capable devices that support "verbs" 39 IBV_QPT_RAW_PACKET QP for Ethernet and/or IBV_QPT_UD QP for IPoIB. 40 . Again. Unwanted dot in line 36, please delete it. 41 This package includes headers for building programs with libvma's interface 42 directly, as opposed to loading it dynamically with LD_PRELOAD. 43 44 %package devel 45 Summary: Header files and link required to develop with Libvma 46 Requires: %{name}%{?_isa} = %{version}-%{release} 47 48 %description devel 49 This package includes headers for building programs with libvma's 50 interfaces. 51 52 %package utils 53 Summary: Libvma utilities 54 Requires: %{name}%{?_isa} = %{version}-%{release} 55 56 %description utils 57 This package contains the tool vma_stats for collecting and 58 analyzing Libvma statistic. 59 60 %prep 61 %setup -q 62 63 %build 64 export revision=1 65 [ -f configure ] || ./autogen.sh Maybe, we should remove the test of file configure . As automake, autoconf and libtool are required as BuildRequires. 66 67 # Debug binary with extra output verbosity 68 %if "%{use_extralib}" == "1" 69 %configure --enable-opt-log=none \ 70 %{?configure_options} 71 %{pmake} please replace %{pmake} with %make_build 72 cp -f src/vma/.libs/%{name}.so %{name}-debug.so It is better to replace 'cp' with 'install'. And it seems duplicated of line 95. 73 %{pmake} clean please replace %{pmake} with %make_build 74 %endif 75 76 # Primary installation set 77 %configure --docdir=%{_docdir}/%{name}-%{version} \ ^^^^^^^^^^ Please don't install doc in versioned dir. See https://fedoraproject.org//wiki/Changes/UnversionedDocdirs 78 %{?configure_options} 79 %{pmake} please replace %{pmake} with %make_build 80 81 %install 82 mkdir -p $RPM_BUILD_ROOT%{_includedir}/mellanox 83 mkdir -p $RPM_BUILD_ROOT%{_sysconfdir} 84 mkdir -p $RPM_BUILD_ROOT%{_libdir} line 83 and 84 is redundance, should be removed. 85 86 %{pmake} DESTDIR=${RPM_BUILD_ROOT} install 87 88 rm -f $RPM_BUILD_ROOT%{_libdir}/*.la find $RPM_BUILD_ROOT -f -name '*.la' -delete 89 90 install -m 644 src/vma/vma_extra.h $RPM_BUILD_ROOT/%{_includedir}/mellanox/vma_extra.h 91 install -m 644 src/vma/util/libvma.conf $RPM_BUILD_ROOT/%{_sysconfdir}/ 92 install -s -m 755 src/stats/vma_stats $RPM_BUILD_ROOT/%{_bindir}/vma_stats 93 install -s -m 755 tools/daemon/vmad $RPM_BUILD_ROOT/%{_sbindir}/vmad 94 %if "%{use_extralib}" == "1" 95 install -m 755 ./%{name}-debug.so $RPM_BUILD_ROOT/%{_libdir}/%{name}-debug.so 96 %endif 97 %if "%{use_systemd}" == "1" 98 install -D -m 644 contrib/scripts/vma.service $RPM_BUILD_ROOT/%{_unitdir}/vma.service 99 install -m 755 contrib/scripts/vma.init $RPM_BUILD_ROOT/%{_sbindir}/vma 100 %else 101 install -m 755 contrib/scripts/vma.init $RPM_BUILD_ROOT/%{_sysconfdir}/init.d/vma 102 %endif please delete line 97, 100 - 102, as use_systemd always true for Fedora. 103 104 %post 105 if [ `grep memlock /etc/security/limits.conf |grep unlimited |wc -l` -le 0 ]; then The test in line 105 is not robust. It is really easy to foo it. Should we replace it with 'ulimit -m' ? 106 echo "* - memlock unlimited" >> /etc/security/limits.conf 107 echo "* soft memlock unlimited" >> /etc/security/limits.conf 108 echo "* hard memlock unlimited" >> /etc/security/limits.conf 109 echo "- Changing max locked memory to unlimited (in /etc/security/limits.conf)" 110 echo " Please log out from the shell and login again in order to update this change " 111 echo " Read more about this topic in the VMA's User Manual" 112 fi 113 /sbin/ldconfig Please see https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets 114 115 # Package setup, not upgrade Please don't include 'comment' in scriptlets. They are not processed as you expected. It will trigger error message. 116 if [ $1 = 1 ]; then 117 if type systemctl >/dev/null 2>&1; then 118 systemctl --no-reload enable vma.service >/dev/null 2>&1 || true 119 elif [ -e /sbin/chkconfig ]; then 120 /sbin/chkconfig --add vma 121 elif [ -e /usr/sbin/update-rc.d ]; then 122 /usr/sbin/update-rc.d vma defaults 123 else 124 %{_libdir}/lsb/install_initd %{_sysconfdir}/init.d/vma 125 fi 126 fi line 114 - 126 should be re-wrote. see https://docs.fedoraproject.org/en-US/packaging-guidelines/Scriptlets/#_scriptlets 127 128 %preun 129 # Package removal, not upgrade 130 if [ $1 = 0 ]; then 131 if type systemctl >/dev/null 2>&1; then 132 systemctl --no-reload disable vma.service >/dev/null 2>&1 || true 133 systemctl stop vma.service || true 134 elif [ -e /sbin/chkconfig ]; then 135 %{_sysconfdir}/init.d/vma stop 136 /sbin/chkconfig --del vma 137 elif [ -e /usr/sbin/update-rc.d ]; then 138 %{_sysconfdir}/init.d/vma stop 139 /usr/sbin/update-rc.d -f vma remove 140 else 141 %{_sysconfdir}/init.d/vma stop 142 %{_libdir}/lsb/remove_initd %{_sysconfdir}/init.d/vma 143 fi 144 fi 145 same as line 114 - 126. 146 %postun 147 # Package upgrade 148 /sbin/ldconfig 149 if type systemctl >/dev/null 2>&1; then 150 systemctl --system daemon-reload >/dev/null 2>&1 || true 151 fi same as line 114 - 126. 152 153 %files 154 %{_libdir}/%{name}.so* 155 %doc %{_docdir}/%{name}-%{version}/README.txt 156 %doc %{_docdir}/%{name}-%{version}/journal.txt 157 %doc %{_docdir}/%{name}-%{version}/VMA_VERSION No versioned doc. 158 %config(noreplace) %{_sysconfdir}/libvma.conf 159 %config(noreplace) %{_sysconfdir}/security/limits.d/30-libvma-limits.conf 160 %{_sbindir}/vmad 161 %if "%{use_systemd}" == "1" 162 %{_prefix}/lib/systemd/system/vma.service 163 %{_sbindir}/vma 164 %else 165 %{_sysconfdir}/init.d/vma 166 %endif use_systemd always true. 167 %if 0%{?rhel} >= 7 || 0%{?fedora} >= 24 || 0%{?suse_version} >= 1500 168 %license COPYING 169 %endif delete line 167, 169. 170 171 %files devel 172 %{_includedir}/mellanox/vma_extra.h 173 %if "%{use_extralib}" == "1" 174 %{_libdir}/%{name}-debug.so 175 %endif 176 177 %files utils 178 %{_bindir}/vma_stats Please create a man page for executable 'vma_stats'.
Thank you for review libvma.spec has been updated accordingly. There are a couple of notes: 1. After applying changes to meet https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets requirements rpmlint reports error: libvma.x86_64: E: postin-without-ldconfig /usr/lib64/libvma.so.9.0.2 This package contains a library and its %post scriptlet doesn't call ldconfig. libvma.x86_64: E: postun-without-ldconfig /usr/lib64/libvma.so.9.0.2 This package contains a library and its %postun doesn't call ldconfig. I am using rpmlint version 1.10 on fc30 2. I would like to avoid putting man page for executable 'vma_stats' at the moment unless it must be done.
(In reply to igor.ivanov.va from comment #5) > Thank you for review > > libvma.spec has been updated accordingly. > > There are a couple of notes: > 1. After applying changes to meet > https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets > requirements > rpmlint reports error: > libvma.x86_64: E: postin-without-ldconfig /usr/lib64/libvma.so.9.0.2 > This package contains a library and its %post scriptlet doesn't call > ldconfig. > > libvma.x86_64: E: postun-without-ldconfig /usr/lib64/libvma.so.9.0.2 > This package contains a library and its %postun doesn't call ldconfig. > That's OK, as it is a rpmlint bug. > I am using rpmlint version 1.10 on fc30 > > 2. I would like to avoid putting man page for executable 'vma_stats' at the > moment unless it must be done. Yes, it is necessary. Please try search with key words " Man page scan results" in bugzilla. It gives 441 bugs for me. That means 441 bugs had been opened to handle manpage like this. https://pagure.io/ManualPageScan/ $ rpm -qpl libvma*rpm | grep bin /usr/sbin/vma /usr/sbin/vmad /usr/bin/vma_stats /usr/sbin/vma is a bash script. It is OK to ignore manpage for it. But vmad and vma_stats are executables, we should provide manpage for them.
$ cat -n libvma.spec ..... 4 %{!?make_build: %global make_build %{__make} %{?_smp_mflags} %{?mflags} V=1} It was my bad. I should told you to delete line 4. 5 %{!?_pkgdocdir: %global _pkgdocdir %{_docdir}/%{name}-%{version}} Why you think line 5 is necessary? Can we just delete it too? 40 This package includes headers for building programs with libvma's interface 41 directly, as opposed to loading it dynamically with LD_PRELOAD. Line 40 and 41 are in the base sub-package libvma's %description section. The base sub-package dose not include any header. It seems line 40 and 41 belongs to sub-package libvma-devel's %description section. Is it right? 52 Summary: Libvma utilities 57 analyzing Libvma statistic. Please replace "Libvma" with "libvma" for line 52 and 57. 68 %configure --docdir=%{_pkgdocdir} \ ^^^^^^^^^^^^^^^^^^^^^^ "--docdir=%{_pkgdocdir}" is unnecessary. Should be removed? 85 %{?ldconfig} 92 %{?ldconfig} Why you need line 85 and 92? 105 %license COPYING Should append "LICENSE" in line 105. $ tar -tf libvma-9.0.2.tar.gz | grep -i license libvma-9.0.2/LICENSE Thanks
Functional smoke test verified libvma works as expected. Performance significantly improved. +------------------+---------------+ |Socket over IPoIB |6.97 Gb/s | +------------------+---------------+ |libvma |24.4 Gb/s | +------------------+---------------+ [root@rdma-dev-21 ~]$ ibstat CA 'ibp130s0f1' CA type: MT4115 Number of ports: 1 Firmware version: 12.23.1020 Hardware version: 0 Node GUID: 0x248a07030049d4f1 System image GUID: 0x248a07030049d4f0 Port 1: State: Active Physical state: LinkUp Rate: 56 Base lid: 32 LMC: 0 SM lid: 1 Capability mask: 0x2659e848 Port GUID: 0x248a07030049d4f1 Link layer: InfiniBand CA 'rocep4s0' CA type: MT4115 Number of ports: 1 Firmware version: 12.23.1020 Hardware version: 0 Node GUID: 0x248a0703004bf094 System image GUID: 0x248a0703004bf094 Port 1: State: Active Physical state: LinkUp Rate: 100 Base lid: 0 LMC: 0 SM lid: 0 Capability mask: 0x00010000 Port GUID: 0x268a07fffe4bf094 Link layer: Ethernet CA 'ibp130s0f0' CA type: MT4115 Number of ports: 1 Firmware version: 12.23.1020 Hardware version: 0 Node GUID: 0x248a07030049d4f0 System image GUID: 0x248a07030049d4f0 Port 1: State: Active Physical state: LinkUp Rate: 56 Base lid: 15 LMC: 0 SM lid: 13 Capability mask: 0x2659e848 Port GUID: 0x248a07030049d4f0 Link layer: InfiniBand [root@rdma-dev-21 ~]$ ip addr show mlx5_ib0 7: mlx5_ib0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc mq state UP group default qlen 256 link/infiniband 00:00:0e:27:fe:80:00:00:00:00:00:00:24:8a:07:03:00:49:d4:f0 brd 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff inet 172.31.0.121/24 brd 172.31.0.255 scope global dynamic noprefixroute mlx5_ib0 valid_lft 2861sec preferred_lft 2861sec inet6 fe80::268a:703:49:d4f0/64 scope link noprefixroute valid_lft forever preferred_lft forever [root@rdma-dev-21 ~]$ iperf -s ------------------------------------------------------------ Server listening on TCP port 5001 TCP window size: 128 KByte (default) ------------------------------------------------------------ [ 4] local 172.31.0.121 port 5001 connected with 172.31.0.122 port 58464 [ ID] Interval Transfer Bandwidth [ 4] 0.0-10.0 sec 8.12 GBytes 6.97 Gbits/sec [root@rdma-dev-22 ~]$ iperf -c 172.31.0.121 ------------------------------------------------------------ Client connecting to 172.31.0.121, TCP port 5001 TCP window size: 612 KByte (default) ------------------------------------------------------------ [ 3] local 172.31.0.122 port 58464 connected with 172.31.0.121 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-10.0 sec 8.12 GBytes 6.98 Gbits/sec [root@rdma-dev-21 ~]$ VMA_SPEC=latency LD_PRELOAD=libvma.so iperf -s VMA INFO: --------------------------------------------------------------------------- VMA INFO: VMA_VERSION: 9.0.2-0 Release built on Apr 17 2020 00:00:00 VMA INFO: Cmd Line: iperf -s VMA INFO: --------------------------------------------------------------------------- VMA INFO: VMA Spec Latency [VMA_SPEC] VMA INFO: Log Level INFO [VMA_TRACELEVEL] VMA INFO: Ring On Device Memory TX 16384 [VMA_RING_DEV_MEM_TX] VMA INFO: Tx QP WRE 256 [VMA_TX_WRE] VMA INFO: Tx QP WRE Batching 4 [VMA_TX_WRE_BATCHING] VMA INFO: Rx QP WRE 256 [VMA_RX_WRE] VMA INFO: Rx QP WRE Batching 4 [VMA_RX_WRE_BATCHING] VMA INFO: Rx Poll Loops -1 [VMA_RX_POLL] VMA INFO: Rx Prefetch Bytes Before Poll 256 [VMA_RX_PREFETCH_BYTES_BEFORE_POLL] VMA INFO: GRO max streams 0 [VMA_GRO_STREAMS_MAX] VMA INFO: Select Poll (usec) -1 [VMA_SELECT_POLL] VMA INFO: Select Poll OS Force Enabled [VMA_SELECT_POLL_OS_FORCE] VMA INFO: Select Poll OS Ratio 1 [VMA_SELECT_POLL_OS_RATIO] VMA INFO: Select Skip OS 1 [VMA_SELECT_SKIP_OS] VMA INFO: CQ Drain Interval (msec) 100 [VMA_PROGRESS_ENGINE_INTERVAL] VMA INFO: CQ Interrupts Moderation Disabled [VMA_CQ_MODERATION_ENABLE] VMA INFO: CQ AIM Max Count 128 [VMA_CQ_AIM_MAX_COUNT] VMA INFO: CQ Adaptive Moderation Disabled [VMA_CQ_AIM_INTERVAL_MSEC] VMA INFO: CQ Keeps QP Full Disabled [VMA_CQ_KEEP_QP_FULL] VMA INFO: TCP nodelay 1 [VMA_TCP_NODELAY] VMA INFO: Avoid sys-calls on tcp fd Enabled [VMA_AVOID_SYS_CALLS_ON_TCP_FD] VMA INFO: Internal Thread Affinity 0 [VMA_INTERNAL_THREAD_AFFINITY] VMA INFO: Thread mode Single [VMA_THREAD_MODE] VMA INFO: --------------------------------------------------------------------------- ------------------------------------------------------------ Server listening on TCP port 5001 TCP window size: 128 KByte (default) ------------------------------------------------------------ [ 53] local 172.31.0.121 port 5001 connected with 172.31.0.122 port 59351 [ ID] Interval Transfer Bandwidth [ 53] 0.0- 9.7 sec 28.4 GBytes 25.1 Gbits/sec [root@rdma-dev-22 ~]$ iperf -c 172.31.0.121 ------------------------------------------------------------ Client connecting to 172.31.0.121, TCP port 5001 TCP window size: 612 KByte (default) ------------------------------------------------------------ [ 3] local 172.31.0.122 port 58464 connected with 172.31.0.121 port 5001 [ ID] Interval Transfer Bandwidth [ 3] 0.0-10.0 sec 8.12 GBytes 6.98 Gbits/sec [root@rdma-dev-22 ~]$ VMA_SPEC=latency LD_PRELOAD=libvma.so iperf -c 172.31.0.121 VMA INFO: --------------------------------------------------------------------------- VMA INFO: VMA_VERSION: 9.0.2-0 Release built on Apr 17 2020 00:00:00 VMA INFO: Cmd Line: iperf -c 172.31.0.121 VMA INFO: --------------------------------------------------------------------------- VMA INFO: VMA Spec Latency [VMA_SPEC] VMA INFO: Log Level INFO [VMA_TRACELEVEL] VMA INFO: Ring On Device Memory TX 16384 [VMA_RING_DEV_MEM_TX] VMA INFO: Tx QP WRE 256 [VMA_TX_WRE] VMA INFO: Tx QP WRE Batching 4 [VMA_TX_WRE_BATCHING] VMA INFO: Rx QP WRE 256 [VMA_RX_WRE] VMA INFO: Rx QP WRE Batching 4 [VMA_RX_WRE_BATCHING] VMA INFO: Rx Poll Loops -1 [VMA_RX_POLL] VMA INFO: Rx Prefetch Bytes Before Poll 256 [VMA_RX_PREFETCH_BYTES_BEFORE_POLL] VMA INFO: GRO max streams 0 [VMA_GRO_STREAMS_MAX] VMA INFO: Select Poll (usec) -1 [VMA_SELECT_POLL] VMA INFO: Select Poll OS Force Enabled [VMA_SELECT_POLL_OS_FORCE] VMA INFO: Select Poll OS Ratio 1 [VMA_SELECT_POLL_OS_RATIO] VMA INFO: Select Skip OS 1 [VMA_SELECT_SKIP_OS] VMA INFO: CQ Drain Interval (msec) 100 [VMA_PROGRESS_ENGINE_INTERVAL] VMA INFO: CQ Interrupts Moderation Disabled [VMA_CQ_MODERATION_ENABLE] VMA INFO: CQ AIM Max Count 128 [VMA_CQ_AIM_MAX_COUNT] VMA INFO: CQ Adaptive Moderation Disabled [VMA_CQ_AIM_INTERVAL_MSEC] VMA INFO: CQ Keeps QP Full Disabled [VMA_CQ_KEEP_QP_FULL] VMA INFO: TCP nodelay 1 [VMA_TCP_NODELAY] VMA INFO: Avoid sys-calls on tcp fd Enabled [VMA_AVOID_SYS_CALLS_ON_TCP_FD] VMA INFO: Internal Thread Affinity 0 [VMA_INTERNAL_THREAD_AFFINITY] VMA INFO: Thread mode Single [VMA_THREAD_MODE] VMA INFO: --------------------------------------------------------------------------- ------------------------------------------------------------ Client connecting to 172.31.0.121, TCP port 5001 TCP window size: 0.00 Byte (default) ------------------------------------------------------------ [ 25] local 172.31.0.122 port 59351 connected with 172.31.0.121 port 5001 [ ID] Interval Transfer Bandwidth [ 25] 0.0-10.0 sec 28.4 GBytes 24.4 Gbits/sec
1 Package Review 2 ============== 3 4 Legend: 5 [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated 6 [ ] = Manual review needed 7 8 9 Issues: 10 ======= 11 - Development (unversioned) .so files in -devel subpackage, if present. 12 Note: Unversioned so-files directly in %_libdir. 13 See: https://docs.fedoraproject.org/en-US/packaging- 14 guidelines/#_devel_packages That's OK, as the libvma-9.0.2-1.fc33.x86_64.rpm/usr/lib64/libvma.so file is used as PRE_LOAD. So, it should be included in the main sub-package. 15 - Package does not use a name that already exists. 16 Note: A package with this name already exists. Please check 17 https://src.fedoraproject.org/rpms/libvma 18 See: https://docs.fedoraproject.org/en-US/packaging- 19 guidelines/Naming/#_conflicting_package_names It's OK, as we are import libvma again for fedora. See https://bugzilla.redhat.com/show_bug.cgi?id=1826439#c3 . 20 21 22 ===== MUST items ===== 23 24 C/C++: 25 [ ]: Package does not contain kernel modules. PASS 26 [ ]: Package contains no static executables. PASS 27 [x]: If your application is a C or C++ application you must list a 28 BuildRequires against gcc, gcc-c++ or clang. 29 [x]: Header files in -devel subpackage, if present. 30 [x]: ldconfig not called in %post and %postun for Fedora 28 and later. 31 [x]: Package does not contain any libtool archives (.la) 32 [x]: Rpath absent or only used for internal libs. 33 34 Generic: 35 [ ]: Package is licensed with an open-source compatible license and meets 36 other legal requirements as defined in the legal section of Packaging 37 Guidelines. PASS 38 [ ]: License field in the package spec file matches the actual license. 39 Note: Checking patched sources after %prep for licenses. Licenses 40 found: "Unknown or generated", "BSD (unspecified) GPL (v2)", "BSD 41 3-clause "New" or "Revised" License", "GPL (v3 or later)", "BSD 42 2-clause "Simplified" License GPL (v2)", "Expat License". 113 files 43 have unknown license. Detailed output of licensecheck in 44 /home/honli/fedora/libvma/1826439-libvma/licensecheck.txt PASS 45 [ ]: License file installed when any subpackage combination is installed. PASS. 46 [ ]: If the package is under multiple licenses, the licensing breakdown 47 must be documented in the spec. PASS 48 [ ]: Package requires other packages for directories it uses. PASS 49 Note: No known owner of /usr/share/doc/libvma, /usr/include/mellanox Please fix this. /usr/share/doc/libvma should be owned by sub-package "libvma". /usr/include/mellanox should be owned by sub-package "libvma-devel". 50 [ ]: Package must own all directories that it creates. 51 Note: Directories without known owners: /etc/security/limits.d, 52 /etc/security, /usr/include/mellanox, /usr/share/doc/libvma PASS. The first two directories are co-owned. The last two are duplicated of line 49. 53 [ ]: %build honors applicable compiler flags or justifies otherwise. PASS 54 [ ]: Package contains no bundled libraries without FPC exception. PASS 55 [ ]: Changelog in prescribed format. PASS 56 [ ]: Sources contain only permissible code or content. PASS 57 [ ]: Package contains desktop file if it is a GUI application. PASS. Not GUI application. 58 [ ]: Development files must be in a -devel package PASS 59 [ ]: Package uses nothing in %doc for runtime. PASS 60 [ ]: Package consistently uses macros (instead of hard-coded directory 61 names). Except the doc dir. Others looks good. 62 [ ]: Package is named according to the Package Naming Guidelines. PASS 63 [ ]: Package does not generate any conflict. PASS 64 [ ]: Package obeys FHS, except libexecdir and /usr/target. PASS 65 [ ]: If the package is a rename of another package, proper Obsoletes and 66 Provides are present. PASS 67 [ ]: Requires correct, justified where necessary. PASS 68 [ ]: Spec file is legible and written in American English. PASS 69 [ ]: Package contains systemd file(s) if in need. PASS. Yes, it does. 70 [ ]: Useful -debuginfo package or justification otherwise. PASS 71 [ ]: Package is not known to require an ExcludeArch tag. PASS, see inline comments in spec file. 72 [ ]: Large documentation must go in a -doc subpackage. Large could be size 73 (~1MB) or number of files. 74 Note: Documentation size is 184320 bytes in 3 files. PASS 75 [ ]: Package complies to the Packaging Guidelines PASS 76 [x]: Package successfully compiles and builds into binary rpms on at least 77 one supported primary architecture. 78 [x]: Package installs properly. 79 [x]: Rpmlint is run on all rpms the build produces. 80 Note: There are rpmlint messages (see attachment). 81 [x]: If (and only if) the source package includes the text of the 82 license(s) in its own file, then that file, containing the text of the 83 license(s) for the package is included in %license. 84 [x]: Package does not own files or directories owned by other packages. 85 [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT 86 [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the 87 beginning of %install. 88 [x]: %config files are marked noreplace or the reason is justified. 89 [x]: Macros in Summary, %description expandable at SRPM build time. 90 [x]: Dist tag is present. 91 [x]: Package does not contain duplicates in %files. 92 [x]: Permissions on files are set properly. 93 [x]: Package must not depend on deprecated() packages. 94 [x]: Package use %makeinstall only when make install DESTDIR=... doesn't 95 work. 96 [x]: Package is named using only allowed ASCII characters. 97 [x]: No %config files under /usr. 98 [x]: Package is not relocatable. 99 [x]: Sources used to build the package match the upstream source, as 100 provided in the spec URL. 101 [x]: Spec file name must match the spec package %{name}, in the format 102 %{name}.spec. 103 [x]: systemd_post is invoked in %post, systemd_preun in %preun, and 104 systemd_postun in %postun for Systemd service files. 105 Note: Systemd service file(s) in libvma 106 [x]: File names are valid UTF-8. 107 [x]: Packages must not store files under /srv, /opt or /usr/local 108 109 ===== SHOULD items ===== 110 111 Generic: 112 [ ]: If the source package does not include license text(s) as a separate 113 file from upstream, the packager SHOULD query upstream to include it. PASS 114 [ ]: Final provides and requires are sane (see attachments). PASS 115 [ ]: Package functions as described. PASS. See https://bugzilla.redhat.com/show_bug.cgi?id=1826439#c8 116 [ ]: Latest version is packaged. PASS 117 [ ]: Package does not include license text files separate from upstream. PASS 118 [ ]: Sources are verified with gpgverify first in %prep if upstream 119 publishes signatures. 120 Note: gpgverify is not used. PASS. NO GPG. 121 [ ]: Description and summary sections in the package spec file contains 122 translations for supported Non-English languages, if available. PASS 123 [ ]: %check is present and all tests pass. PASS 124 [ ]: Packages should try to preserve timestamps of original installed 125 files. PASS 126 [x]: Reviewer should test that the package builds in mock. 127 [x]: Buildroot is not present 128 [x]: Package has no %clean section with rm -rf %{buildroot} (or 129 $RPM_BUILD_ROOT) 130 [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. 131 [x]: Fully versioned dependency in subpackages if applicable. 132 [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file 133 [x]: Sources can be downloaded from URI in Source: tag 134 [x]: SourceX is a working URL. 135 [x]: Package should compile and build into binary rpms on all supported 136 architectures. 137 [x]: Spec use %global instead of %define unless justified. 138 139 ===== EXTRA items ===== 140 141 Generic: 142 [!]: Package should not use obsolete m4 macros 143 Note: Some obsoleted macros found, see the attachment. 144 See: https://fedorahosted.org/FedoraReview/wiki/AutoTools Please fix this. 145 [x]: Rpmlint is run on debuginfo package(s). 146 Note: No rpmlint messages. 147 [x]: Rpmlint is run on all installed packages. 148 Note: There are rpmlint messages (see attachment). 149 [x]: Large data in /usr/share should live in a noarch subpackage if package 150 is arched. 151 [x]: Spec file according to URL is the same as in SRPM. 152 153 154 Rpmlint 155 ------- 156 Checking: libvma-9.0.2-1.fc33.x86_64.rpm 157 libvma-devel-9.0.2-1.fc33.x86_64.rpm 158 libvma-utils-9.0.2-1.fc33.x86_64.rpm 159 libvma-debuginfo-9.0.2-1.fc33.x86_64.rpm 160 libvma-debugsource-9.0.2-1.fc33.x86_64.rpm 161 libvma-9.0.2-1.fc33.src.rpm 162 libvma.x86_64: W: shared-lib-calls-exit /usr/lib64/libvma.so.9.0.2 exit.5 163 libvma.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libvma.so 164 libvma.x86_64: E: postin-without-ldconfig /usr/lib64/libvma.so.9.0.2 165 libvma.x86_64: E: postun-without-ldconfig /usr/lib64/libvma.so.9.0.2 line 164 and 165 are false positive. 166 libvma.x86_64: W: no-manual-page-for-binary vma 167 libvma.x86_64: W: no-manual-page-for-binary vmad should be fixed. 168 libvma-devel.x86_64: W: no-documentation 169 libvma-utils.x86_64: W: spelling-error %description -l en_US vma -> ma, via, v ma 170 libvma-utils.x86_64: W: no-documentation 171 libvma-utils.x86_64: W: no-manual-page-for-binary vma_stats should be fixed. 172 6 packages and 0 specfiles checked; 2 errors, 8 warnings. 173 174 175 176 177 Rpmlint (debuginfo) 178 ------------------- 179 Checking: libvma-debuginfo-9.0.2-1.fc33.x86_64.rpm 180 1 packages and 0 specfiles checked; 0 errors, 0 warnings. 181 182 183 184 185 186 Rpmlint (installed packages) 187 ---------------------------- 188 libvma-debugsource.x86_64: W: invalid-url URL: https://github.com/Mellanox/libvma <urlopen error [Errno -2] Name or service not known> 189 libvma.x86_64: W: invalid-url URL: https://github.com/Mellanox/libvma <urlopen error [Errno -2] Name or service not known> 190 libvma.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libvma.so.9.0.2 /lib64/librt.so.1 191 libvma.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libvma.so.9.0.2 /lib64/libm.so.6 192 libvma.x86_64: W: shared-lib-calls-exit /usr/lib64/libvma.so.9.0.2 exit.5 193 libvma.x86_64: W: devel-file-in-non-devel-package /usr/lib64/libvma.so 194 libvma.x86_64: E: postin-without-ldconfig /usr/lib64/libvma.so.9.0.2 195 libvma.x86_64: E: postun-without-ldconfig /usr/lib64/libvma.so.9.0.2 196 libvma.x86_64: W: no-manual-page-for-binary vma 197 libvma.x86_64: W: no-manual-page-for-binary vmad 198 libvma-debuginfo.x86_64: W: invalid-url URL: https://github.com/Mellanox/libvma <urlopen error [Errno -2] Name or service not known> 199 libvma-utils.x86_64: W: spelling-error %description -l en_US vma -> ma, via, v ma 200 libvma-utils.x86_64: W: invalid-url URL: https://github.com/Mellanox/libvma <urlopen error [Errno -2] Name or service not known> 201 libvma-utils.x86_64: W: no-documentation 202 libvma-utils.x86_64: W: no-manual-page-for-binary vma_stats 203 libvma-devel.x86_64: W: invalid-url URL: https://github.com/Mellanox/libvma <urlopen error [Errno -2] Name or service not known> 204 libvma-devel.x86_64: W: no-documentation 205 5 packages and 0 specfiles checked; 2 errors, 15 warnings. duplicated error report. 206 207 208 209 Unversioned so-files 210 -------------------- 211 libvma: /usr/lib64/libvma.so 212 213 Source checksums 214 ---------------- 215 https://github.com/Mellanox/libvma/archive/9.0.2/libvma-9.0.2.tar.gz : 216 CHECKSUM(SHA256) this package : 1f273c309f553bd479da229a39b93d53fcf3fdda9b8eae2df973f6b8d02aa164 217 CHECKSUM(SHA256) upstream package : 1f273c309f553bd479da229a39b93d53fcf3fdda9b8eae2df973f6b8d02aa164 218 219 220 Requires 221 -------- 222 libvma (rpmlib, GLIBC filtered): 223 /bin/sh 224 /usr/bin/bash 225 config(libvma) 226 ld-linux-x86-64.so.2()(64bit) 227 libc.so.6()(64bit) 228 libdl.so.2()(64bit) 229 libgcc_s.so.1()(64bit) 230 libgcc_s.so.1(GCC_3.0)(64bit) 231 libibverbs.so.1()(64bit) 232 libibverbs.so.1(IBVERBS_1.0)(64bit) 233 libibverbs.so.1(IBVERBS_1.1)(64bit) 234 libibverbs.so.1(IBVERBS_1.8)(64bit) 235 libm.so.6()(64bit) 236 libmlx5.so.1()(64bit) 237 libmlx5.so.1(MLX5_1.2)(64bit) 238 libmlx5.so.1(MLX5_1.4)(64bit) 239 libnl-3.so.200()(64bit) 240 libnl-3.so.200(libnl_3)(64bit) 241 libnl-route-3.so.200()(64bit) 242 libnl-route-3.so.200(libnl_3)(64bit) 243 libpthread.so.0()(64bit) 244 librdmacm.so.1()(64bit) 245 librdmacm.so.1(RDMACM_1.0)(64bit) 246 librt.so.1()(64bit) 247 libstdc++.so.6()(64bit) 248 libstdc++.so.6(CXXABI_1.3)(64bit) 249 libstdc++.so.6(CXXABI_1.3.8)(64bit) 250 libstdc++.so.6(CXXABI_1.3.9)(64bit) 251 libvma.so.9()(64bit) 252 rtld(GNU_HASH) 253 254 libvma-devel (rpmlib, GLIBC filtered): 255 libvma(x86-64) 256 257 libvma-utils (rpmlib, GLIBC filtered): 258 libc.so.6()(64bit) 259 libgcc_s.so.1()(64bit) 260 libgcc_s.so.1(GCC_3.0)(64bit) 261 libpthread.so.0()(64bit) 262 libstdc++.so.6()(64bit) 263 libstdc++.so.6(CXXABI_1.3)(64bit) 264 libstdc++.so.6(CXXABI_1.3.9)(64bit) 265 libvma(x86-64) 266 rtld(GNU_HASH) 267 268 libvma-debuginfo (rpmlib, GLIBC filtered): 269 270 libvma-debugsource (rpmlib, GLIBC filtered): 271 272 273 274 Provides 275 -------- 276 libvma: 277 config(libvma) 278 libvma 279 libvma(x86-64) 280 libvma.so.9()(64bit) 281 282 libvma-devel: 283 libvma-devel 284 libvma-devel(x86-64) 285 286 libvma-utils: 287 libvma-utils 288 libvma-utils(x86-64) 289 290 libvma-debuginfo: 291 debuginfo(build-id) 292 libvma-debuginfo 293 libvma-debuginfo(x86-64) 294 295 libvma-debugsource: 296 libvma-debugsource 297 libvma-debugsource(x86-64) 298 299 300 301 AutoTools: Obsoleted m4s found 302 ------------------------------ 303 AC_PROG_LIBTOOL found in: libvma-9.0.2/configure.ac:107 Please fix this. 304 305 306 Generated by fedora-review 0.7.5 (5fa5b7e) last change: 2020-02-16 307 Command line :/usr/bin/fedora-review -b 1826439 308 Buildroot used: fedora-rawhide-x86_64 309 Active plugins: Generic, C/C++, Shell-api 310 Disabled plugins: fonts, SugarActivity, Haskell, Ocaml, PHP, Perl, Java, R, Python, Ruby 311 Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH Thanks
I added patch with man pages (https://raw.githubusercontent.com/igor-ivanov/libvma/pr/fedora-package/fedora/0001-issue-928161-Add-man-pages.patch) and updated source rpm with spec. Thanks, Igor
(In reply to igor.ivanov.va from comment #10) > I added patch with man pages > (https://raw.githubusercontent.com/igor-ivanov/libvma/pr/fedora-package/ > fedora/0001-issue-928161-Add-man-pages.patch) I checked libvma git repo. I did not found this patch. The patch should be applied in upstream before apply it for fedora. Please see https://docs.fedoraproject.org/en-US/packaging-guidelines/#_patch_guidelines .
Ok, I will update you after getting pacth into master.
(In reply to igor.ivanov.va from comment #10) > I added patch with man pages > (https://raw.githubusercontent.com/igor-ivanov/libvma/pr/fedora-package/ > fedora/0001-issue-928161-Add-man-pages.patch) > and > updated source rpm with spec. The updated spec file looks good to me. I will set the fedora-review+ flag, after the patch get into upstream. Thanks
The patch is a part of master as https://github.com/Mellanox/libvma/commit/b9aabc058b0524aadff9c756abaf57b660cec63b Thanks, Igor
Please tell me your FSA account. I searched 'igor.ivanov.va' in https://src.fedoraproject.org/users , but did not get a match.
May FCA : iivanov
@mschmidt, could you please add iivanov into packager group?
Michal, Honggang what is next step for getting one into fc33? Thanks, Igor
(In reply to igor.ivanov.va from comment #18) > Michal, Honggang > what is next step for getting one into fc33? https://fedoraproject.org/wiki/Orphaned_package_that_need_new_maintainers Thanks
Added https://pagure.io/releng/issue/9509 Thanks, Igor
Hi Igor, I have not added you to the packagers group yet. I have some questions and comments about the package. libvma.spec: > BuildRequires: libibverbs-devel > BuildRequires: rdma-core-devel These days, libibverbs-devel is provided by rdma-core-devel. > BuildRequires: libnl3-devel Your config/m4/nl.m4 uses PKG_CHECK_MODULES, so you should BR pkgconfig(libnl-route-3.0) instead. https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/ > %build > export revision=%{use_rel} This seems unnecessary. I did not find anything referencing the "revision" environment variable in your build system. > %install > %{make_build} DESTDIR=${RPM_BUILD_ROOT} install You should use %{make_install} instead. > %files > %{_libdir}/%{name}.so* So the package contains something like this: libvma.so -> libvma.so.9.1.0 libvma.so.9 -> libvma.so.9.1.0 libvma.so.9.1.0 Since libvma.so is to be used only for LD_PRELOAD and no program should be linked against it (and have DT_NEEDED for it),: 1. is there a point in versioning the library? 2. does it have to be installed in the linker's default path? Would it make more sense to have it as %{_libdir}/libvma/libvma.so ? Do you actually use configure_options for anything? I don't think it's common for Fedora packages to have macros like that "just in case". use_rel - This is fascinating. The macro not only determines the value of the Release tag, it also influences the build where its value gets used for version compatibility checks and encoded in version strings. To me the mechanism seems needlessly complicated and I fail to see the benefit of it. Also, are you aware that sometimes automatic scripts (e.g. release engineering mass rebuilds) bump the Release fields in Fedora spec files? Normally in RPM, Version is the upstream version and Release serves packaging needs. 30-libvma-limits.conf: > # Default limits that are needed for proper work of libvma > # Read more about this topic in the VMA's User Manual Can you point me to where in the manual is this discussed? > * - memlock unlimited > * soft memlock unlimited > * hard memlock unlimited Does having the package installed give every user on the system the permission to mlock unlimited amounts of RAM? Is that really necessary? Could it be at least limited to a user group? vma.service: > [Unit] > Description=VMA Daemon. Version: 9.1.0-0 Please consider dropping the version from the Description. No other service has that. > After=network.target syslog.target syslog.target became irrelevant and was removed in 2013. Please remove that dependency. I am aware some packages still have that, but it's just cargo-cult now. > Requires=network.target This does not make sense for your daemon. See man systemd.special(7) about the intended use of network.target. > [Service] > Type=forking > Restart=on-failure > ExecStart=/usr/sbin/vma start > ExecStop=/usr/sbin/vma stop /usr/sbin/vma is a SysV initscript. That's terrible. Why not start the deamon binary directly?: ExecStart=/usr/sbin/vmad (And maybe set KillSignal if needed.) > ExecReload=/usr/sbin/vma restart You must not fake the reload operation with restart. If the service cannot do a reload, just do not define ExecReload. > RestartForceExitStatus=1 SIGTERM It's unusual to need to use this setting. There may be a good reason for it, but please double check.
Hi Michal, I have updated libvma.spec to meet most of your requirements and added additional patch as https://raw.githubusercontent.com/igor-ivanov/libvma/pr/fedora-package/fedora/0002-Update-systemctl-files.patch For others please look at my comments inline. Thanks, Igor (In reply to Michal Schmidt from comment #21) > Hi Igor, > I have not added you to the packagers group yet. I have some questions and > comments about the package. > > libvma.spec: > > BuildRequires: libibverbs-devel > > BuildRequires: rdma-core-devel > Done > These days, libibverbs-devel is provided by rdma-core-devel. > > > BuildRequires: libnl3-devel > > Your config/m4/nl.m4 uses PKG_CHECK_MODULES, so you should BR > pkgconfig(libnl-route-3.0) instead. > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > PkgConfigBuildRequires/ > Done > > %build > > export revision=%{use_rel} > > This seems unnecessary. I did not find anything referencing the "revision" > environment variable in your build system. > It is used during configure to set VMA_SVN_REVISION. > > %install > > %{make_build} DESTDIR=${RPM_BUILD_ROOT} install > > You should use %{make_install} instead. > Corrected but following rules as https://docs.fedoraproject.org/en-US/packaging-guidelines/#_why_the_makeinstall_macro_should_not_be_used permit original usage. > > %files > > %{_libdir}/%{name}.so* > > So the package contains something like this: > libvma.so -> libvma.so.9.1.0 > libvma.so.9 -> libvma.so.9.1.0 > libvma.so.9.1.0 > > Since libvma.so is to be used only for LD_PRELOAD and no program should be > linked against it (and have DT_NEEDED for it),: > 1. is there a point in versioning the library? > 2. does it have to be installed in the linker's default path? Would it make > more sense to have it as %{_libdir}/libvma/libvma.so ? > livma has own extra api with vma_extra.h header file and admits direct linkage. > > Do you actually use configure_options for anything? I don't think it's > common for Fedora packages to have macros like that "just in case". > This option is used to provide flexibility in final binary libvma version. For example: this option can be used to enable tso support (--enable-tso) that increases throughput but not ready to be included by default. > use_rel - This is fascinating. The macro not only determines the value of > the Release tag, it also influences the build where its value gets used for > version compatibility checks and encoded in version strings. To me the > mechanism seems needlessly complicated and I fail to see the benefit of it. > Also, are you aware that sometimes automatic scripts (e.g. release > engineering mass rebuilds) bump the Release fields in Fedora spec files? > Normally in RPM, Version is the upstream version and Release serves > packaging needs. > I removed it but it is used in pair with configure_options to differentiate packages for the customers on our side. > 30-libvma-limits.conf: > > # Default limits that are needed for proper work of libvma > > # Read more about this topic in the VMA's User Manual > > Can you point me to where in the manual is this discussed? > > > * - memlock unlimited > > * soft memlock unlimited > > * hard memlock unlimited > > Does having the package installed give every user on the system the > permission to mlock unlimited amounts of RAM? Is that really necessary? > Could it be at least limited to a user group? > There is such recommendation at https://docs.mellanox.com/display/VMAv902/VMA+Installation+Options that forces to https://github.com/Mellanox/libvma/wiki/VMA-over-RHEL-7.x-with-inbox-driver#4-common-configurations > vma.service: > > [Unit] > > Description=VMA Daemon. Version: 9.1.0-0 > > Please consider dropping the version from the Description. No other service > has that. > Done > > After=network.target syslog.target > > syslog.target became irrelevant and was removed in 2013. Please remove that > dependency. I am aware some packages still have that, but it's just > cargo-cult now. > > > Requires=network.target > > This does not make sense for your daemon. See man systemd.special(7) about > the intended use of network.target. > Done > > [Service] > > Type=forking > > Restart=on-failure > > ExecStart=/usr/sbin/vma start > > ExecStop=/usr/sbin/vma stop > > /usr/sbin/vma is a SysV initscript. That's terrible. > Why not start the deamon binary directly?: > ExecStart=/usr/sbin/vmad > (And maybe set KillSignal if needed.) > It does support of some systems easier for us. > > ExecReload=/usr/sbin/vma restart > > You must not fake the reload operation with restart. If the service cannot > do a reload, just do not define ExecReload. > > > RestartForceExitStatus=1 SIGTERM > > It's unusual to need to use this setting. There may be a good reason for it, > but please double check. SIGTERM is generated by kill utility by default. So it is used to restart vmad in this case too.
(In reply to igor.ivanov.va from comment #22) > > > %build > > > export revision=%{use_rel} > > > > This seems unnecessary. I did not find anything referencing the "revision" > > environment variable in your build system. > > > > It is used during configure to set VMA_SVN_REVISION. Oh, I see. I was looking at the master branch, where it's not used anymore, but it is indeed used in the version you packaged. > > > %install > > > %{make_build} DESTDIR=${RPM_BUILD_ROOT} install > > > > You should use %{make_install} instead. > > > > Corrected but following rules as > https://docs.fedoraproject.org/en-US/packaging-guidelines/ > #_why_the_makeinstall_macro_should_not_be_used permit original usage. The current guidelines allow this form: make DESTDIR=$RPM_BUILD_ROOT install but that's not the same as what you originally had: %{make_build} DESTDIR=${RPM_BUILD_ROOT} install There's been a recent push to convert all Fedora spec files to %make_install: https://lists.fedoraproject.org/archives/list/devel@lists.fedoraproject.org/message/IPPMIU6DS664TCWTLBR7WA2CUAOFUD7M/ Thanks for having changed it to %{make_install}. > > > %files > > > %{_libdir}/%{name}.so* > > > > So the package contains something like this: > > libvma.so -> libvma.so.9.1.0 > > libvma.so.9 -> libvma.so.9.1.0 > > libvma.so.9.1.0 > > > > Since libvma.so is to be used only for LD_PRELOAD and no program should be > > linked against it (and have DT_NEEDED for it),: > > 1. is there a point in versioning the library? > > 2. does it have to be installed in the linker's default path? Would it make > > more sense to have it as %{_libdir}/libvma/libvma.so ? > > livma has own extra api with vma_extra.h header file and admits direct > linkage. I didn't see such usage in the documentation, but I have now tested linking a C program with libvma directly and it seems to work, so OK. > > Do you actually use configure_options for anything? I don't think it's > > common for Fedora packages to have macros like that "just in case". > > This option is used to provide flexibility in final binary libvma version. OK, it does no harm. It can stay. > > 30-libvma-limits.conf: > > > # Default limits that are needed for proper work of libvma > > > # Read more about this topic in the VMA's User Manual > > > * - memlock unlimited > > > * soft memlock unlimited > > > * hard memlock unlimited > > > > Does having the package installed give every user on the system the > > permission to mlock unlimited amounts of RAM? Is that really necessary? > > Could it be at least limited to a user group? > > There is such recommendation at > https://docs.mellanox.com/display/VMAv902/VMA+Installation+Options that > forces to > https://github.com/Mellanox/libvma/wiki/VMA-over-RHEL-7.x-with-inbox- > driver#4-common-configurations Thanks for the pointer, but it does not answer the question why the memlock resource limit has to be lifted for all users by default. Have other options been considered? Could it be limited to a user group? i.e.: @libvmauser - memlock unlimited > > > [Service] > > > Type=forking > > > Restart=on-failure > > > ExecStart=/usr/sbin/vma start > > > ExecStop=/usr/sbin/vma stop > > > > /usr/sbin/vma is a SysV initscript. That's terrible. > > Why not start the deamon binary directly?: > > ExecStart=/usr/sbin/vmad > > (And maybe set KillSignal if needed.) > > It does support of some systems easier for us. Please elaborate. What systems? How exactly does running a SysV script from a systemd service help? I see the SysV script performs work that systemd already does by itself (checking whether the service is already running, finding a process to kill, reporting status). And it does it less accurately than systemd (which tracks services by cgroups). It also adds some "sleep 1", which is either pointless, or papering over a race condition bug. I tried running the daemon directly using "ExecStart=/usr/sbin/vmad" it the unit file. To my surprise the daemon exits immediately when run this way. This is because of this code in tools/daemon/daemon.c:main(): /* already a daemon */ if (getppid() == 1) { return 0; } That should be removed. Daemons should not change behaviour depending on which process spawned them. > > > RestartForceExitStatus=1 SIGTERM > > > > It's unusual to need to use this setting. There may be a good reason for it, > > but please double check. > > SIGTERM is generated by kill utility by default. So it is used to restart > vmad in this case too. Yes, kill sends SIGTERM by default. I don't understand how the second sentence follows. If I kill a service with SIGTERM, presumably I want it to terminate, don't I?
(In reply to Michal Schmidt from comment #23) > > > 30-libvma-limits.conf: > > > > # Default limits that are needed for proper work of libvma > > > > # Read more about this topic in the VMA's User Manual > > > > * - memlock unlimited > > > > * soft memlock unlimited > > > > * hard memlock unlimited > > > > > > Does having the package installed give every user on the system the > > > permission to mlock unlimited amounts of RAM? Is that really necessary? > > > Could it be at least limited to a user group? > > > > There is such recommendation at > > https://docs.mellanox.com/display/VMAv902/VMA+Installation+Options that > > forces to > > https://github.com/Mellanox/libvma/wiki/VMA-over-RHEL-7.x-with-inbox- > > driver#4-common-configurations > > Thanks for the pointer, but it does not answer the question why the memlock > resource limit has to be lifted for all users by default. Have other options > been considered? Could it be limited to a user group? i.e.: > @libvmauser - memlock unlimited In real HPC production environment, we are always to lifted the resource for all users. For example, see "13.5.3. Relaxing memlock restrictions for users" https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/networking_guide/sec-configuring_the_base_rdma_subsystem
(In reply to Honggang LI from comment #24) > In real HPC production environment, we are always to lifted the resource for > all users. For example, see "13.5.3. Relaxing memlock restrictions for users" > > https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/ > html/networking_guide/sec-configuring_the_base_rdma_subsystem Do we ship this config file in a package? It's one thing to suggest administrators to configure their systems in a certain way. It still requires a concious decision on their part. It's another thing to ship this config in a package.
(In reply to Michal Schmidt from comment #25) > (In reply to Honggang LI from comment #24) > > In real HPC production environment, we are always to lifted the resource for > > all users. For example, see "13.5.3. Relaxing memlock restrictions for users" > > > > https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/ > > html/networking_guide/sec-configuring_the_base_rdma_subsystem > > Do we ship this config file in a package? No, but we setup it with kickstart scripts for our internal cluster. [root@rdma-dev-01 ~]$ cat /etc/security/limits.d/rdma.conf # configuration for rdma performance tuning * soft memlock unlimited * hard memlock unlimited * soft stack unlimited * hard stack unlimited * soft core unlimited * hard core unlimited # rdma tuning end > It's one thing to suggest administrators to configure their systems in a > certain way. It still requires a concious decision on their part. > It's another thing to ship this config in a package.
(In reply to Michal Schmidt from comment #23) > (In reply to igor.ivanov.va from comment #22) > > > > > [Service] > > > > Type=forking > > > > Restart=on-failure > > > > ExecStart=/usr/sbin/vma start > > > > ExecStop=/usr/sbin/vma stop > > > > > > /usr/sbin/vma is a SysV initscript. That's terrible. > > > Why not start the deamon binary directly?: > > > ExecStart=/usr/sbin/vmad > > > (And maybe set KillSignal if needed.) > > > > It does support of some systems easier for us. > > Please elaborate. What systems? How exactly does running a SysV script from > a systemd service help? > I see the SysV script performs work that systemd already does by itself > (checking whether the service is already running, finding a process to kill, > reporting status). And it does it less accurately than systemd (which tracks > services by cgroups). It also adds some "sleep 1", which is either > pointless, or papering over a race condition bug. > > I tried running the daemon directly using "ExecStart=/usr/sbin/vmad" it the > unit file. To my surprise the daemon exits immediately when run this way. > This is because of this code in tools/daemon/daemon.c:main(): > > /* already a daemon */ > if (getppid() == 1) { > return 0; > } > > That should be removed. Daemons should not change behaviour depending on > which process spawned them. > It seems that I did not understand you initial question but I see your point now. I will look and back with answer. > > > > RestartForceExitStatus=1 SIGTERM > > > > > > It's unusual to need to use this setting. There may be a good reason for it, > > > but please double check. > > > > SIGTERM is generated by kill utility by default. So it is used to restart > > vmad in this case too. > > Yes, kill sends SIGTERM by default. I don't understand how the second > sentence follows. If I kill a service with SIGTERM, presumably I want it to > terminate, don't I? You are correct. Current behaviour was done basing on customer's requests that want to stop daemon just using systemctl and avoid stopping one by tool like kill, pkill.
Thank you for review Package was updated. Changes include: 1. 30-libvma-limits.conf was removed (let administrator configure system basing on internal requirements for user, groups etc. libvma has run-time check as https://github.com/igor-ivanov/libvma/blob/master/src/vma/main.cpp#L423) 2. vma.service started using vmad directly.
Hello, Do you see other issue? Thanks, Igor
Igor, I can see no more blocking issues. Thanks! I have added you to the "packager" Fedora group. You can reopen https://pagure.io/releng/issue/9509 now.
Thank you
Hi, We've reopened the ticket to Unretire libvma: https://pagure.io/releng/issue/9509. Thanks, Alex
(In reply to alexv from comment #32) > We've reopened the ticket to Unretire libvma: > https://pagure.io/releng/issue/9509. The ticket is closed now. Please build latest upstream release libvma for Fedora Rawhide. It is better to build libvma for f32 and f31 too. Thanks
Hi Honggang, Could you advise how to create remote f31, f32 branches for libvma. I can not do it useing `git push origin f32` Thanks, Igor
$ cd "$LIBVMA_REPO_DIR" $ fedpkg request-branch "$BRANCH_NAME" or, alternatively: $ fedpkg request-branch --repo "rpms/libvma" "$BRANCH_NAME"
I submitted request to unblock f31 as https://pagure.io/releng/issue/9681
Please create fedora update for libvma-9.0.2-1.fc32 and libvma build for F31. Install bodhi-client and run 'bodhi updates new --help' for documentation.
libvma-9.0.2-1.fc33 and libvma-9.0.2-1.fc34 had been built. Close it.
(In reply to Honggang LI from comment #37) > Please create fedora update for libvma-9.0.2-1.fc32 and libvma build for F31. > > Install bodhi-client and run 'bodhi updates new --help' for documentation. I used "fedpkg update" for this w/o success. It looks as common issue these days (for example: https://www.spinics.net/lists/fedora-devel/msg276677.html).
Please use bodhi to create update for libvma. I confirmed it works for me. fabtests (f32)]$ bodhi updates new --type bugfix --notes "Update fabtests to latest upstream release v1.11.0" --user honli fabtests-1.11.0-1.fc32 ================================================================================ fabtests-1.11.0-1.fc32 ================================================================================ Update ID: FEDORA-2020-2b79a2faad Content Type: rpm Release: Fedora 32 Status: pending Type: bugfix Severity: unspecified Karma: 0 Autokarma: False [-3, 3] CI Status: no tests are required Request: testing Notes: Update fabtests to latest upstream release v1.11.0 Submitter: honli Submitted: 2020-08-17 09:15:32 Comments: bodhi - 2020-08-17 09:15:32 (karma 0) This update has been submitted for testing by honli. bodhi - 2020-08-17 09:15:32 (karma 0) This update's test gating status has been changed to 'ignored'. bodhi - 2020-08-17 09:15:32 (karma 0) This update's test gating status has been changed to 'waiting'. https://bodhi.fedoraproject.org/updates/FEDORA-2020-2b79a2faad Caveats: The number of stable days required was set to the mandatory release value of 7 days
Thank you, Honggang. It works. f31: https://bodhi.fedoraproject.org/updates/FEDORA-2020-cc2738fbff f32: https://bodhi.fedoraproject.org/updates/FEDORA-2020-32cff33f00