| Message ID | 20240228181409.3756293-1-sairon@sairon.cz |
|---|---|
| State | Superseded |
| Headers | show |
| Series | package/nfs-utils: only install fsidd binary and unit file with enabled nfsd | expand |
On 28. 02. 24 19:14, Jan Čermák wrote: > Unit file for the FSID daemon depends on the nfs-server.service, which is > removed without BR2_PACKAGE_NFS_UTILS_RPC_NFSD enabled. Also don't install > the fsidd service binary without nfsd enabled. Also, I *think* that sqlite (and libevent?) dependencies could be dropped in the case when nfsd (or nfsdcld/nfsdcltrack) is not installed. But there are no configure flags for this scenario in the upstream, nfsd is simply removed after the install in BR, so this would either need bit more elaborate patching or upstream coordination. Maybe Giulio or Petr can give some insights here. Anyway, let me know what you think and if it's worth pursuing this. Cheers, Jan
Hi Jan, thank you for the contribution, On 28/02/24 19:14, Jan Čermák wrote: > Unit file for the FSID daemon depends on the nfs-server.service, which is > removed without BR2_PACKAGE_NFS_UTILS_RPC_NFSD enabled. Also don't install > the fsidd service binary without nfsd enabled. regarding the commit log(I'm not that good at it but I give a try) what about: ``` FSID daemon and its systemd unit file both depend on BR2_PACKAGE_NFS_UTILS_RPC_NFSD at the moment they are installed in any case. So let's remove them both when BR2_PACKAGE_NFS_UTILS_RPC_NFSD is disabled. ``` > > Signed-off-by: Jan Čermák <sairon@sairon.cz> For the rest it looks good to me, I've build tested it and it works as expected so: Reviewed-by: Giulio Benetti <giulio.benetti@benettiengineering.com> Can you please send a V2 with improved commit log? Thank you Best regards
Hi Ja, On 28/02/24 19:23, Jan Čermák wrote: > On 28. 02. 24 19:14, Jan Čermák wrote: >> Unit file for the FSID daemon depends on the nfs-server.service, which is >> removed without BR2_PACKAGE_NFS_UTILS_RPC_NFSD enabled. Also don't >> install >> the fsidd service binary without nfsd enabled. > > Also, I *think* that sqlite (and libevent?) dependencies could be > dropped in the case when nfsd (or nfsdcld/nfsdcltrack) is not installed. > But there are no configure flags for this scenario in the upstream, nfsd > is simply removed after the install in BR, so this would either need bit > more elaborate patching or upstream coordination. Maybe Giulio or Petr > can give some insights here. > > Anyway, let me know what you think and if it's worth pursuing this. Checking nfs-utils configure.ac I see that at the moment both libevent and sqlite3 are required [0]. I haven't digged enough if RPC_NFSD really requires those 2 dependencies, but if you think they don't requite it you have first of all to patch configure.ac to avoid checking for them in any case and then you have to deeper and finally see if it really doesn't need that. To be honest, the way the package is handled at the moment is not the best way to go. I mean, building something we don't need and later removing it it's an option, but not the best one. It would be great if you could provide a patches for nfs-utils to allow disabling rpcdebug, rpc.lockd and rpc.nfsd, rpc.quotad and send them upstream. This way we could drop these lines [1] and this line as well [2] and handle everything as done for GSS [3] That would be a good starting point. Then later you could more easily move dependencies inside configure.ac, but for the moment if you remove libevent and sqlite from nfs-utils package dependencies it will fail to build, because it will fail on configure.ac and in any case it will try to build all the daemons I've listed above that have dependencies. So there's more work to be done but if dependencies you've found are correct then yes, it can be done. Best regards [0]: https://git.linux-nfs.org/?p=steved/nfs-utils.git;a=blob;f=configure.ac;h=58d1728c5bc6a7928548514d56837eea3b6e91cd;hb=HEAD#l344 [1]: https://gitlab.com/buildroot.org/buildroot/-/blob/master/package/nfs-utils/nfs-utils.mk?ref_type=heads#L41-45 [2]: https://gitlab.com/buildroot.org/buildroot/-/blob/master/package/nfs-utils/nfs-utils.mk?ref_type=heads#L72 [3]: https://gitlab.com/buildroot.org/buildroot/-/blob/master/package/nfs-utils/nfs-utils.mk?ref_type=heads#L54-62
Hi Giulio, On 29. 02. 24 22:31, Giulio Benetti wrote: > For the rest it looks good to me, I've build tested it and it works > as expected so: > Reviewed-by: Giulio Benetti <giulio.benetti@benettiengineering.com> > > Can you please send a V2 with improved commit log? thanks for the review! I'll send the updated patch right away. Cheers, Jan
diff --git a/package/nfs-utils/nfs-utils.mk b/package/nfs-utils/nfs-utils.mk index 4f2b41c782..b757e3e51d 100644 --- a/package/nfs-utils/nfs-utils.mk +++ b/package/nfs-utils/nfs-utils.mk @@ -42,7 +42,8 @@ NFS_UTILS_TARGETS_$(BR2_PACKAGE_NFS_UTILS_RPCDEBUG) += usr/sbin/rpcdebug NFS_UTILS_TARGETS_$(BR2_PACKAGE_NFS_UTILS_RPC_LOCKD) += usr/sbin/rpc.lockd NFS_UTILS_TARGETS_$(BR2_PACKAGE_NFS_UTILS_RPC_RQUOTAD) += usr/sbin/rpc.rquotad NFS_UTILS_TARGETS_$(BR2_PACKAGE_NFS_UTILS_RPC_NFSD) += usr/sbin/exportfs \ - usr/sbin/rpc.mountd usr/sbin/rpc.nfsd usr/lib/systemd/system/nfs-server.service + usr/sbin/rpc.mountd usr/sbin/rpc.nfsd usr/lib/systemd/system/nfs-server.service \ + usr/sbin/fsidd usr/lib/systemd/system/fsidd.service ifeq ($(BR2_PACKAGE_NFS_UTILS_NFSV4),y) NFS_UTILS_CONF_OPTS += --enable-nfsv4 --enable-nfsv41
Unit file for the FSID daemon depends on the nfs-server.service, which is removed without BR2_PACKAGE_NFS_UTILS_RPC_NFSD enabled. Also don't install the fsidd service binary without nfsd enabled. Signed-off-by: Jan Čermák <sairon@sairon.cz> --- package/nfs-utils/nfs-utils.mk | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)