Add config to nfsd to set number of threads

Buildroot Andreas Ehmanns 2 participants 4 messages Dec 10, 2015 – Dec 17, 2015

Share & tools

Andreas Ehmanns

author

2015-12-10 19:34 UTC #0 lore raw

Dear All,
actually the nfs-utils package provides an NFS server together with an init script. The number of threads the NFS server shall use is hard-coded in the init script. The attached patch moves this parameter to a config file under /etc which is read by the init script.

Additionally the return value of the init script in case of an not existing file has been changed from 0 to 2 to indicate an error condition.

What do you think?

Regards,
Andreas

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Make-number-of-nfsd-threads-configurable.patch
Type: text/x-patch
Size: 2216 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151210/66a34828/attachment.bin>

Yann E. MORIN

2015-12-10 20:17 UTC #1 lore raw

Andreas, All,

On 2015-12-10 20:34 +0100, Andreas Ehmanns spake thusly:

Quoted 5 lines

Dear All,
actually the nfs-utils package provides an NFS server together with an init script. The number of threads the NFS server shall use is hard-coded in the init script. The attached patch moves this parameter to a config file under /etc which is read by the init script.

The idea is OK with me. However, I have a few comments...

Quoted 2 lines

Additionally the return value of the init script in case of an not existing file has been changed from 0 to 2 to indicate an error condition.

We don't care about the value of the exit status of a SysV init script. Whether it returns 0, 1, 2 or whatever else is completely ignored by /sbin/init .

Quoted 19 lines

From f1eae31bb357587c3491a1b45891dda8c6c0dbff Mon Sep 17 00:00:00 2001
From: Andreas Ehmanns <[email protected]>
Date: Thu, 10 Dec 2015 15:51:23 +0100
Subject: [PATCH 1/1] Make number of nfsd threads configurable
Signed-off-by Andreas Ehmanns <[email protected]>
---
 package/nfs-utils/S60nfs       | 13 ++++++++-----
 package/nfs-utils/nfs-utils.mk |  2 ++
 package/nfs-utils/nfsd.conf    |  8 ++++++++
 3 files changed, 18 insertions(+), 5 deletions(-)
 create mode 100644 package/nfs-utils/nfsd.conf
package/nfs-utils/S60nfs +0 -0
index ec7c909..ddc472e 100755
@@ -3,10 +3,13 @@
 # nfs           This shell script takes care of starting and stopping

package/nfs-utils/S60nfs line 9

Quoted 10 lines

package/nfs-utils/S60nfs +4 -4

 #               the NFS services. Stolen from RedHat FC5.
 
-[ -x /usr/sbin/rpc.statd ] || exit 0
-[ -x /usr/sbin/rpc.nfsd ] || exit 0
-[ -x /usr/sbin/rpc.mountd ] || exit 0
-[ -x /usr/sbin/exportfs ] || exit 0
+[ -x /usr/sbin/rpc.statd ] || exit 2
+[ -x /usr/sbin/rpc.nfsd ] || exit 2
+[ -x /usr/sbin/rpc.mountd ] || exit 2
+[ -x /usr/sbin/exportfs ] || exit 2

Do we really need to check for those files? nfs-utils has no config option for any of those progs, only for: rpc.lockd and rpc.rquotad, but we're not using them in this init script.

This means that we can assume that all the utilities above *are* installed when the init script is.

package/nfs-utils/S60nfs line 11

Quoted 2 lines

package/nfs-utils/S60nfs +2 -0
+[ -r /etc/nfsd.conf ] || exit 2
+. /etc/nfsd.conf

Two comments about that:

- move the file to /etc/default/nfsd
- do not make it mandatory and just provide defaults
- use a variable to store the path
- no need to prefix the variable with 'NFSD'

(yes, that's four comments! ;-) )

So, what about:

    CFG_FILE=/etc/default/nfsd

    NR_THREADS=2
    if [ -f "${CFG_FILE}" ]; then
        . "${CFG_FILE}"
    fi

package/nfs-utils/S60nfs start() { · line 31

Quoted 8 lines

package/nfs-utils/S60nfs +1 -1

 mkdir -p /var/lock/subsys
 mkdir -p /run/nfs/sm

@@ -25,7 +28,7 @@ start() {
 	echo "done"
 
 	printf "Starting NFS daemon: "
-	rpc.nfsd 2
+	rpc.nfsd $NFSD_NR_OF_THREADS

I have a preference for using ${foo} when expanding variables, so;

    rpc.nfsd ${NR_THREADS}

package/nfs-utils/nfsd.conf

Quoted 18 lines

package/nfs-utils/S60nfs +0 -0

 	echo "done"
 
 	printf "Starting NFS mountd: "

package/nfs-utils/nfs-utils.mk +2 -0

index 30f12fd..a26c626 100644
@@ -51,6 +51,8 @@ endif
 define NFS_UTILS_INSTALL_INIT_SYSV
 	$(INSTALL) -D -m 0755 package/nfs-utils/S60nfs \
 		$(TARGET_DIR)/etc/init.d/S60nfs
+	$(INSTALL) -m 644 package/nfs-utils/nfsd.conf \
+		$(TARGET_DIR)/etc/nfsd.conf
 endef
 
 define NFS_UTILS_INSTALL_INIT_SYSTEMD

package/nfs-utils/nfsd.conf new file +0 -0

new file mode 100644

[--SNIP--]

That file is no longer needed now that there is a default in the script. Just let the user provide one via a rootfs overlay or a post-install script.

Care to resubmit a proper patch with the above fixed, please? Thanks! :-)

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Andreas Ehmanns

author

2015-12-14 19:24 UTC #2 lore raw

Hi Yann,

Am 10.12.2015 um 21:17 schrieb Yann E. MORIN:

Quoted 104 lines

Andreas, All,
On 2015-12-10 20:34 +0100, Andreas Ehmanns spake thusly:
Quoted 5 lines
Dear All,
actually the nfs-utils package provides an NFS server together with an init script. The number of threads the NFS server shall use is hard-coded in the init script. The attached patch moves this parameter to a config file under /etc which is read by the init script.
The idea is OK with me. However, I have a few comments...
Quoted 2 lines
Additionally the return value of the init script in case of an not existing file has been changed from 0 to 2 to indicate an error condition.
We don't care about the value of the exit status of a SysV init script. Whether it returns 0, 1, 2 or whatever else is completely ignored by /sbin/init .
Quoted 19 lines
From f1eae31bb357587c3491a1b45891dda8c6c0dbff Mon Sep 17 00:00:00 2001
From: Andreas Ehmanns <[email protected]>
Date: Thu, 10 Dec 2015 15:51:23 +0100
Subject: [PATCH 1/1] Make number of nfsd threads configurable
Signed-off-by Andreas Ehmanns <[email protected]>
---
  package/nfs-utils/S60nfs       | 13 ++++++++-----
  package/nfs-utils/nfs-utils.mk |  2 ++
  package/nfs-utils/nfsd.conf    |  8 ++++++++
  3 files changed, 18 insertions(+), 5 deletions(-)
create mode 100644 package/nfs-utils/nfsd.conf
package/nfs-utils/S60nfs +0 -0
index ec7c909..ddc472e 100755
@@ -3,10 +3,13 @@
  # nfs           This shell script takes care of starting and stopping
package/nfs-utils/S60nfs line 9
Quoted 10 lines
package/nfs-utils/S60nfs +4 -4
  #               the NFS services. Stolen from RedHat FC5.
 
-[ -x /usr/sbin/rpc.statd ] || exit 0
-[ -x /usr/sbin/rpc.nfsd ] || exit 0
-[ -x /usr/sbin/rpc.mountd ] || exit 0
-[ -x /usr/sbin/exportfs ] || exit 0
+[ -x /usr/sbin/rpc.statd ] || exit 2
+[ -x /usr/sbin/rpc.nfsd ] || exit 2
+[ -x /usr/sbin/rpc.mountd ] || exit 2
+[ -x /usr/sbin/exportfs ] || exit 2
Do we really need to check for those files? nfs-utils has no config option for any of those progs, only for: rpc.lockd and rpc.rquotad, but we're not using them in this init script.
This means that we can assume that all the utilities above *are* installed when the init script is.
package/nfs-utils/S60nfs line 11
Quoted 2 lines
package/nfs-utils/S60nfs +2 -0
+[ -r /etc/nfsd.conf ] || exit 2
+. /etc/nfsd.conf
Two comments about that:
- move the file to /etc/default/nfsd
- do not make it mandatory and just provide defaults
- use a variable to store the path
- no need to prefix the variable with 'NFSD'
(yes, that's four comments! ;-) )
So, what about:
     CFG_FILE=/etc/default/nfsd

     NR_THREADS=2
     if [ -f "${CFG_FILE}" ]; then
         . "${CFG_FILE}"
     fi
package/nfs-utils/S60nfs start() { · line 31
Quoted 8 lines
package/nfs-utils/S60nfs +1 -1
  mkdir -p /var/lock/subsys
  mkdir -p /run/nfs/sm
@@ -25,7 +28,7 @@ start() {
  	echo "done"
 
  	printf "Starting NFS daemon: "
-	rpc.nfsd 2
+	rpc.nfsd $NFSD_NR_OF_THREADS
I have a preference for using ${foo} when expanding variables, so;
     rpc.nfsd ${NR_THREADS}
package/nfs-utils/nfsd.conf
Quoted 18 lines
package/nfs-utils/S60nfs +0 -0
  	echo "done"
 
  	printf "Starting NFS mountd: "
package/nfs-utils/nfs-utils.mk +2 -0
index 30f12fd..a26c626 100644
@@ -51,6 +51,8 @@ endif
  define NFS_UTILS_INSTALL_INIT_SYSV
  	$(INSTALL) -D -m 0755 package/nfs-utils/S60nfs \
  		$(TARGET_DIR)/etc/init.d/S60nfs
+	$(INSTALL) -m 644 package/nfs-utils/nfsd.conf \
+		$(TARGET_DIR)/etc/nfsd.conf
  endef
 
  define NFS_UTILS_INSTALL_INIT_SYSTEMD
package/nfs-utils/nfsd.conf new file +0 -0
new file mode 100644
[--SNIP--]

Quoted 9 lines

That file is no longer needed now that there is a default in the script. Just let the user provide one via a rootfs overlay or a post-install script.
Care to resubmit a proper patch with the above fixed, please? Thanks! :-)
Regards,
Yann E. MORIN.

sounds good to me. I will prepare a patch and send via git this week.

Regards,
Andreas

Andreas Ehmanns

author

2015-12-17 19:49 UTC #3 lore raw

In reply to Yann E. MORIN

Yann, All,
Am 10.12.2015 um 21:17 schrieb Yann E. MORIN:

Quoted 104 lines

Andreas, All,
On 2015-12-10 20:34 +0100, Andreas Ehmanns spake thusly:
Quoted 5 lines
Dear All,
actually the nfs-utils package provides an NFS server together with an init script. The number of threads the NFS server shall use is hard-coded in the init script. The attached patch moves this parameter to a config file under /etc which is read by the init script.
The idea is OK with me. However, I have a few comments...
Quoted 2 lines
Additionally the return value of the init script in case of an not existing file has been changed from 0 to 2 to indicate an error condition.
We don't care about the value of the exit status of a SysV init script. Whether it returns 0, 1, 2 or whatever else is completely ignored by /sbin/init .
Quoted 19 lines
From f1eae31bb357587c3491a1b45891dda8c6c0dbff Mon Sep 17 00:00:00 2001
From: Andreas Ehmanns <[email protected]>
Date: Thu, 10 Dec 2015 15:51:23 +0100
Subject: [PATCH 1/1] Make number of nfsd threads configurable
Signed-off-by Andreas Ehmanns <[email protected]>
---
  package/nfs-utils/S60nfs       | 13 ++++++++-----
  package/nfs-utils/nfs-utils.mk |  2 ++
  package/nfs-utils/nfsd.conf    |  8 ++++++++
  3 files changed, 18 insertions(+), 5 deletions(-)
create mode 100644 package/nfs-utils/nfsd.conf
package/nfs-utils/S60nfs +0 -0
index ec7c909..ddc472e 100755
@@ -3,10 +3,13 @@
  # nfs           This shell script takes care of starting and stopping
package/nfs-utils/S60nfs line 9
Quoted 10 lines
package/nfs-utils/S60nfs +4 -4
  #               the NFS services. Stolen from RedHat FC5.
 
-[ -x /usr/sbin/rpc.statd ] || exit 0
-[ -x /usr/sbin/rpc.nfsd ] || exit 0
-[ -x /usr/sbin/rpc.mountd ] || exit 0
-[ -x /usr/sbin/exportfs ] || exit 0
+[ -x /usr/sbin/rpc.statd ] || exit 2
+[ -x /usr/sbin/rpc.nfsd ] || exit 2
+[ -x /usr/sbin/rpc.mountd ] || exit 2
+[ -x /usr/sbin/exportfs ] || exit 2
Do we really need to check for those files? nfs-utils has no config option for any of those progs, only for: rpc.lockd and rpc.rquotad, but we're not using them in this init script.
This means that we can assume that all the utilities above *are* installed when the init script is.
package/nfs-utils/S60nfs line 11
Quoted 2 lines
package/nfs-utils/S60nfs +2 -0
+[ -r /etc/nfsd.conf ] || exit 2
+. /etc/nfsd.conf
Two comments about that:
- move the file to /etc/default/nfsd
- do not make it mandatory and just provide defaults
- use a variable to store the path
- no need to prefix the variable with 'NFSD'
(yes, that's four comments! ;-) )
So, what about:
     CFG_FILE=/etc/default/nfsd

     NR_THREADS=2
     if [ -f "${CFG_FILE}" ]; then
         . "${CFG_FILE}"
     fi
package/nfs-utils/S60nfs start() { · line 31
Quoted 8 lines
package/nfs-utils/S60nfs +1 -1
  mkdir -p /var/lock/subsys
  mkdir -p /run/nfs/sm
@@ -25,7 +28,7 @@ start() {
  	echo "done"
 
  	printf "Starting NFS daemon: "
-	rpc.nfsd 2
+	rpc.nfsd $NFSD_NR_OF_THREADS
I have a preference for using ${foo} when expanding variables, so;
     rpc.nfsd ${NR_THREADS}
package/nfs-utils/nfsd.conf
Quoted 18 lines
package/nfs-utils/S60nfs +0 -0
  	echo "done"
 
  	printf "Starting NFS mountd: "
package/nfs-utils/nfs-utils.mk +2 -0
index 30f12fd..a26c626 100644
@@ -51,6 +51,8 @@ endif
  define NFS_UTILS_INSTALL_INIT_SYSV
  	$(INSTALL) -D -m 0755 package/nfs-utils/S60nfs \
  		$(TARGET_DIR)/etc/init.d/S60nfs
+	$(INSTALL) -m 644 package/nfs-utils/nfsd.conf \
+		$(TARGET_DIR)/etc/nfsd.conf
  endef
 
  define NFS_UTILS_INSTALL_INIT_SYSTEMD
package/nfs-utils/nfsd.conf new file +0 -0
new file mode 100644
[--SNIP--]

Quoted 9 lines

That file is no longer needed now that there is a default in the script. Just let the user provide one via a rootfs overlay or a post-install script.
Care to resubmit a proper patch with the above fixed, please? Thanks! :-)
Regards,
Yann E. MORIN.

I picked up your ideas and prepared a new patch. Just sent it via git email two minutes ago. Additionally replaced occurrence of "echo -n" with "printf".

Regards,
Andreas