Bug 189696 - [Regression] mdconfig and mdconfig2 startup scripts
Summary: [Regression] mdconfig and mdconfig2 startup scripts
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Some People
Assignee: Devin Teske
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-12 07:50 UTC by Ganael Laplanche
Modified: 2015-11-02 21:31 UTC (History)
5 users (show)

See Also:


Attachments
file.diff (806 bytes, patch)
2014-05-12 07:50 UTC, Ganael Laplanche
no flags Details | Diff
mdconfig startup patch (898 bytes, patch)
2015-10-29 18:09 UTC, geodni
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ganael Laplanche 2014-05-12 07:50:00 UTC
Hi,

svn rev. 264243 introduced a regression in rc.d/mdconfig and mdconfig2 scripts.

The following configuration (within /etc/rc.conf) :

# Ramdisk
mdconfig_md0="-t swap -s 4096m" 
mdconfig_md0_owner="root:wheel"
mdconfig_md0_perms="1777" 
mdconfig_md0_cmd="mkdir /tmp/wrkdirs"

now leads to errors because of wrong list_vars arguments within the scripts.

With current arguments, the scripts catch too many md devices and now try to create md0, but also md0_owner, md0_perms and md0_cmd.

Fix: Patch attached with submission follows:
How-To-Repeat: Try the configuration above and do :

# /etc/rc.d/mdconfig start
# /etc/rc.d/mdconfig2 start

(or reboot)
Comment 1 Ryan Steinmetz freebsd_committer freebsd_triage 2014-10-25 14:41:56 UTC
# grep mdconf /etc/rc.conf
mdconfig_md0="-t malloc -s 128m -o reserve"
mdconfig_md0_owner="1001"
mdconfig_md0_perms="0700"

# mdconfig restart
Umounting /dev/md0.
Destroying md0.
Device /dev/md0_owner isn't mounted.
Device /dev/md0_perms isn't mounted.
Creating md0 device (malloc).
Mounting /dev/md0.
Creating md0_owner device (1001).
usage: mdconfig -a -t type [-n] [-o [no]option] ... [-f file]
                [-s size] [-S sectorsize] [-u unit]
                [-x sectors/track] [-y heads/cylinder]
       mdconfig -d -u unit [-o [no]force]
       mdconfig -l [-v] [-n] [-f file] [-u unit]
       mdconfig file
                type = {malloc, vnode, swap}
                option = {cluster, compress, reserve}
                size = %d (512 byte blocks), %db (B),
                       %dk (kB), %dm (MB), %dg (GB) or
                       %dt (TB)
Creating md0_owner device failed, moving on.
Creating md0_perms device (0700).
usage: mdconfig -a -t type [-n] [-o [no]option] ... [-f file]
                [-s size] [-S sectorsize] [-u unit]
                [-x sectors/track] [-y heads/cylinder]
       mdconfig -d -u unit [-o [no]force]
       mdconfig -l [-v] [-n] [-f file] [-u unit]
       mdconfig file
                type = {malloc, vnode, swap}
                option = {cluster, compress, reserve}
                size = %d (512 byte blocks), %db (B),
                       %dk (kB), %dm (MB), %dg (GB) or
                       %dt (TB)
Creating md0_perms device failed, moving on.
Comment 2 geodni 2015-10-29 18:09:38 UTC
Created attachment 162576 [details]
mdconfig startup patch
Comment 3 geodni 2015-10-29 18:10:33 UTC
Hello,
The patch is not enough to build the exact list of mds without args. Shell regex is POSIX compliant. You have to restrict the config list with some egrep and takes care of mdconfig devices above 9.
The "mdconfig_md[0-9]\*" regex that will me matching var in "list_vars" matches all and other additionnal parameter for the same mdX. 
Perhaps it might be preferable to get rid of this like it is done for multiple network interfaces from same brand in future or inspire from "ppp_profile" list.
By now the patch attached solved the problem for md0 to md99 and get rids of type mismatch like "mdconfig_mdr10".
Denis
Comment 4 commit-hook freebsd_committer freebsd_triage 2015-10-29 21:13:32 UTC
A commit references this bug:

Author: dteske
Date: Thu Oct 29 21:12:58 UTC 2015
New revision: 290163
URL: https://svnweb.freebsd.org/changeset/base/290163

Log:
  Ignore per-mdN settings in mdconfig[2] startup

  PR:		base/189696
  Submitted by:	ganael.laplanche@martymac.org
  MFC after:	3 days
  X-MFC-to:	stable/10 stable/9

Changes:
  head/etc/rc.d/mdconfig
  head/etc/rc.d/mdconfig2
Comment 5 Devin Teske freebsd_committer freebsd_triage 2015-10-29 21:20:51 UTC
Fixed in HEAD. See SVN r290163. Will merge to stable/10 and stable/9 after 3-day minimum MFC requirement.

Note to Denis...
You can't use grep (nor egrep) in these boot scripts. Reason being two-fold:
1. grep/egrep is in /usr/bin
2. The mdconfig boot script runs before filesystems are mounted (see below)

$ rcorder /etc/rc.d/* 2> /dev/null | grep -n '\(FILE\|mount\|mdc\)'
14:/etc/rc.d/mdconfig
16:/etc/rc.d/mountcritlocal
20:/etc/rc.d/FILESYSTEMS
60:/etc/rc.d/mountcritremote
63:/etc/rc.d/mdconfig2
103:/etc/rc.d/mountd
120:/etc/rc.d/mountlate

NOTE: mountcritlocal is the script that mounts /usr/bin on systems that have legacy partitioning (e.g., UFS with separate /, /usr, /var, and /tmp bsdlabels).

NOTE: In the situation where /usr/bin is part of /, you won't see the problem with using grep/egrep in the mdconfig boot script.

Note to OP...
Original patch could be fooled into thinking mdconfig_md0_setting8 was a unit to allocate (wherein mdconfig would attempt to allocate md0_setting8; the patch in committed SVN rev is not fooled by this and would skip md0_setting8 but catch the mdconfig_md0 setting to cause allocation of md0).
Comment 6 Devin Teske freebsd_committer freebsd_triage 2015-10-29 21:21:05 UTC
Fixed in HEAD. See SVN r290163. Will merge to stable/10 and stable/9 after 3-day minimum MFC requirement.

Note to Denis...
You can't use grep (nor egrep) in these boot scripts. Reason being two-fold:
1. grep/egrep is in /usr/bin
2. The mdconfig boot script runs before filesystems are mounted (see below)

$ rcorder /etc/rc.d/* 2> /dev/null | grep -n '\(FILE\|mount\|mdc\)'
14:/etc/rc.d/mdconfig
16:/etc/rc.d/mountcritlocal
20:/etc/rc.d/FILESYSTEMS
60:/etc/rc.d/mountcritremote
63:/etc/rc.d/mdconfig2
103:/etc/rc.d/mountd
120:/etc/rc.d/mountlate

NOTE: mountcritlocal is the script that mounts /usr/bin on systems that have legacy partitioning (e.g., UFS with separate /, /usr, /var, and /tmp bsdlabels).

NOTE: In the situation where /usr/bin is part of /, you won't see the problem with using grep/egrep in the mdconfig boot script.

Note to OP...
Original patch could be fooled into thinking mdconfig_md0_setting8 was a unit to allocate (wherein mdconfig would attempt to allocate md0_setting8; the patch in committed SVN rev is not fooled by this and would skip md0_setting8 but catch the mdconfig_md0 setting to cause allocation of md0).
Comment 7 Devin Teske freebsd_committer freebsd_triage 2015-10-29 21:22:12 UTC
MFC pending. Closing ahead of MFC.
Comment 8 commit-hook freebsd_committer freebsd_triage 2015-11-02 21:26:32 UTC
A commit references this bug:

Author: dteske
Date: Mon Nov  2 21:26:10 UTC 2015
New revision: 290277
URL: https://svnweb.freebsd.org/changeset/base/290277

Log:
  MFC r290163: Ignore per-mdN settings in mdconfig[2] startup

  PR:		base/189696
  Submitted by:	ganael.laplanche@martymac.org

Changes:
_U  stable/10/
  stable/10/etc/rc.d/mdconfig
  stable/10/etc/rc.d/mdconfig2
Comment 9 commit-hook freebsd_committer freebsd_triage 2015-11-02 21:31:34 UTC
A commit references this bug:

Author: dteske
Date: Mon Nov  2 21:30:53 UTC 2015
New revision: 290278
URL: https://svnweb.freebsd.org/changeset/base/290278

Log:
  MFC r290163: Ignore per-mdN settings in mdconfig[2] startup

  PR:		base/189696
  Submitted by:	ganael.laplanche@martymac.org

Changes:
_U  stable/9/
_U  stable/9/etc/
_U  stable/9/etc/rc.d/
  stable/9/etc/rc.d/mdconfig
  stable/9/etc/rc.d/mdconfig2