From owner-freebsd-rc@FreeBSD.ORG Sun Jun 9 18:13:36 2013 Return-Path: Delivered-To: rc@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id EB925710; Sun, 9 Jun 2013 18:13:36 +0000 (UTC) (envelope-from prvs=18721298a7=killing@multiplay.co.uk) Received: from mail1.multiplay.co.uk (mail1.multiplay.co.uk [85.236.96.23]) by mx1.freebsd.org (Postfix) with ESMTP id 317E51819; Sun, 9 Jun 2013 18:13:35 +0000 (UTC) Received: from r2d2 ([82.69.141.170]) by mail1.multiplay.co.uk (mail1.multiplay.co.uk [85.236.96.23]) (MDaemon PRO v10.0.4) with ESMTP id md50004232324.msg; Sun, 09 Jun 2013 19:13:35 +0100 X-Spam-Processed: mail1.multiplay.co.uk, Sun, 09 Jun 2013 19:13:35 +0100 (not processed: message from valid local sender) X-MDDKIM-Result: neutral (mail1.multiplay.co.uk) X-MDRemoteIP: 82.69.141.170 X-Return-Path: prvs=18721298a7=killing@multiplay.co.uk X-Envelope-From: killing@multiplay.co.uk Message-ID: <5E2843FF4724492FBF2A29E43B698385@multiplay.co.uk> From: "Steven Hartland" To: "Hiroki Sato" References: <20130609.041903.541782050134731313.hrs@allbsd.org> Subject: Re: Feeback on patch to add ZFS support to mdconfig rc.d scripts Date: Sun, 9 Jun 2013 19:13:31 +0100 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_0570_01CE6545.6E1C99C0" X-Priority: 3 X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook Express 6.00.2900.5931 X-MimeOLE: Produced By Microsoft MimeOLE V6.00.2900.6157 Cc: rc@FreeBSD.org, zfs-devel@FreeBSD.org X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 09 Jun 2013 18:13:37 -0000 This is a multi-part message in MIME format. ------=_NextPart_000_0570_01CE6545.6E1C99C0 Content-Type: text/plain; format=flowed; charset="iso-8859-1"; reply-type=original Content-Transfer-Encoding: 7bit ----- Original Message ----- From: "Hiroki Sato" > I think this should be separated into adding non-fs md support into > rc.d/mdconfig and on-the-fly zpool creation/import into rc.d/zfs > since the rc.d/mdconfig script does not (and should not) depend on > zfs.ko module. > > rc.d/mdconfig is located before mountcritlocal, so probably > rc.d/zfs_md or something is needed just after rc.d/mdconfig2. Thanks for that Hiroki. If I understand correctly that your only concern is the zfs module dependency? If so then its easier to make that a dynamic dependency using a prestart. Updated patch attached, which does just that. Does this look better? Regards Steve ================================================ This e.mail is private and confidential between Multiplay (UK) Ltd. and the person or entity to whom it is addressed. In the event of misdirection, the recipient is prohibited from using, copying, printing or otherwise disseminating it or any information contained in it. In the event of misdirection, illegible or incomplete transmission please telephone +44 845 868 1337 or return the E.mail to postmaster@multiplay.co.uk. ------=_NextPart_000_0570_01CE6545.6E1C99C0 Content-Type: application/octet-stream; name="mdconfig-zfs.patch" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="mdconfig-zfs.patch" Add ZFS pool creation support to mdconfig rc.d script=0A= =0A= Fix failure error redirection in mount check=0A= =0A= Fix invalid configuration detection enabling -t vnode=0A= to be skipped if -f is specified as supported=0A= by mdconfig.=0A= =0A= Fix fsck not working if mount point is not present in fstab.=0A= =0A= TODO: update rc.conf man page=0A= --- /etc/rc.d/mdconfig.orig 2013-06-08 00:55:48.599761883 +0000=0A= +++ /etc/rc.d/mdconfig 2013-06-09 18:02:08.037511687 +0000=0A= @@ -35,9 +35,28 @@=0A= name=3D"mdconfig"=0A= stop_cmd=3D"mdconfig_stop"=0A= start_cmd=3D"mdconfig_start"=0A= -start_precmd=3D'[ -n "${_mdconfig_list}" ]'=0A= +start_precmd=3D"mdconfig_prestart"=0A= required_modules=3D"geom_md:g_md"=0A= =0A= +mdconfig_prestart()=0A= +{=0A= + local _mp=0A= +=0A= + if [ -z "${_mdconfig_list}" ]; then=0A= + return 1=0A= + fi=0A= +=0A= + for _md in ${_mdconfig_list}; do=0A= + eval _zpool=3D\$mdconfig_${_md}_zfs_pool=0A= + if [ -n "${_zpool}" ]; then=0A= + required_modules=3D"$required_modules zfs"=0A= + break=0A= + fi=0A= + done=0A= +=0A= + return 0=0A= +}=0A= +=0A= is_readonly()=0A= {=0A= local _mp _ret=0A= @@ -61,6 +80,29 @@=0A= fi=0A= }=0A= =0A= +get_opt()=0A= +{=0A= + local _flag _param _opt=0A= +=0A= + _flag=3D$1=0A= + _param=3D$2=0A= +=0A= + _opt=3D${_config##*-${_flag}\ }=0A= + if [ "${_opt}" =3D "${_config}" ]; then=0A= + _opt=3D""=0A= + else=0A= + _opt=3D${_opt%%\ *}=0A= + fi=0A= + eval _${_param}=3D${_opt}=0A= + debug "${_md} ${_param}=3D${_opt}"=0A= +=0A= + if [ -z "${_opt}" ]; then=0A= + return 0=0A= + fi=0A= +=0A= + return 1=0A= +}=0A= +=0A= init_variables()=0A= {=0A= local _i=0A= @@ -70,16 +112,20 @@=0A= _dev=3D"/dev/${_md}"=0A= eval _config=3D\$mdconfig_${_md}=0A= eval _newfs=3D\$mdconfig_${_md}_newfs=0A= + eval _zpool=3D\$mdconfig_${_md}_zfs_pool=0A= + eval _zflags=3D\$mdconfig_${_md}_zfs_flags=0A= =0A= - _type=3D${_config##*-t\ }=0A= - _type=3D${_type%%\ *}=0A= + get_opt "t" "type"=0A= + get_opt "f" "file"=0A= if [ -z "${_type}" ]; then=0A= - err 1 "You need to specify \"-t \" in mdconfig_${_md}"=0A= + if [ -n "${_file}" ]; then=0A= + _type=3D"vnode"=0A= + else=0A= + err 1 "You need to specify \"-t \" in mdconfig_${_md}"=0A= + fi=0A= fi=0A= =0A= if [ "${_type}" =3D "vnode" ]; then=0A= - _file=3D${_config##*-f\ }=0A= - _file=3D${_file%%\ *}=0A= if [ -z "${_file}" ]; then=0A= err 2 "You need to specify \"-f \" in mdconfig_${_md} for = vnode devices"=0A= fi=0A= @@ -96,11 +142,13 @@=0A= debug "${_md} file: ${_file}"=0A= debug "${_md} fs: ${_fs}"=0A= debug "${_md} newfs flags: ${_newfs}"=0A= + debug "${_md} ZFS pool: ${_zpool}"=0A= + debug "${_md} ZFS flags: ${_zflags}"=0A= }=0A= =0A= mdconfig_start()=0A= {=0A= - local _md _mp _config _type _dev _file _fs _newfs _fsck_cmd=0A= + local _md _mp _config _type _dev _file _fs _newfs _fsck_cmd _zpool = _zflags=0A= =0A= for _md in ${_mdconfig_list}; do=0A= init_variables ${_md}=0A= @@ -126,14 +174,26 @@=0A= echo "Creating ${_md} device failed, moving on."=0A= continue=0A= fi=0A= +=0A= + if [ -n "${_zpool}" ]; then=0A= + if [ "${_type}" =3D "vnode" ]; then=0A= + echo "Importing ZFS pool ${_zpool}."=0A= + zpool import ${_zpool}=0A= + else=0A= + echo "Creating ZFS pool ${_zpool}."=0A= + zpool create ${_zflags} ${_zpool} ${_md}=0A= + fi=0A= + continue=0A= + fi=0A= +=0A= # Skip fsck for uzip devices.=0A= if [ "${_type}" =3D "vnode" ]; then=0A= if [ "${_file}" !=3D "${_file%.uzip}" ]; then=0A= _fsck_cmd=3D":"=0A= elif checkyesno background_fsck; then=0A= - _fsck_cmd=3D"fsck -F"=0A= + _fsck_cmd=3D"fsck -t ufs -F"=0A= else=0A= - _fsck_cmd=3D"fsck"=0A= + _fsck_cmd=3D"fsck -t ufs"=0A= fi=0A= if ! eval ${_fsck_cmd} -p ${_dev} >/dev/null; then=0A= echo "Fsck failed on ${_dev}, not mounting the filesystem."=0A= @@ -143,7 +203,8 @@=0A= else=0A= newfs ${_newfs} ${_dev} >/dev/null=0A= fi=0A= - if mount -d ${_dev} 2>&1 >/dev/null; then=0A= +=0A= + if mount -d ${_dev} >/dev/null 2>&1; then=0A= echo "Mounting ${_dev}."=0A= mount ${_dev}=0A= fi=0A= @@ -159,7 +220,12 @@=0A= init_variables ${_md}=0A= if [ "${_type}" !=3D "vnode" -o "${_fs}" =3D "/" ]; then=0A= for _i in `df ${_dev} 2>/dev/null`; do _mp=3D${_i}; done=0A= - if [ -z "${_mp}" -o "${_mp}" !=3D "${_mp%%%}" ]; then=0A= + if [ -n "${_zpool}" ]; then=0A= + if zpool list ${_zpool} >/dev/null 2>&1; then=0A= + echo "Exporting ZFS pool ${_zpool}."=0A= + zpool export ${_zpool}=0A= + fi=0A= + elif [ -z "${_mp}" -o "${_mp}" !=3D "${_mp%%%}" ]; then=0A= echo "Device ${_dev} isn't mounted."=0A= else=0A= echo "Umounting ${_dev}."=0A= --- /etc/rc.d/mdconfig2.orig 2013-06-08 16:33:49.583875813 +0000=0A= +++ /etc/rc.d/mdconfig2 2013-06-09 18:03:39.055114045 +0000=0A= @@ -36,9 +36,28 @@=0A= name=3D"mdconfig2"=0A= stop_cmd=3D"mdconfig2_stop"=0A= start_cmd=3D"mdconfig2_start"=0A= -start_precmd=3D'[ -n "${_mdconfig2_list}" ]'=0A= +start_precmd=3D"mdconfig2_prestart"=0A= required_modules=3D"geom_md:g_md"=0A= =0A= +mdconfig2_prestart()=0A= +{=0A= + local _mp=0A= +=0A= + if [ -z "${_mdconfig2_list}" ]; then=0A= + return 1=0A= + fi=0A= +=0A= + for _md in ${_mdconfig2_list}; do=0A= + eval _zpool=3D\$mdconfig_${_md}_zfs_pool=0A= + if [ -n "${_zpool}" ]; then=0A= + required_modules=3D"$required_modules zfs"=0A= + break=0A= + fi=0A= + done=0A= +=0A= + return 0=0A= +}=0A= +=0A= is_readonly()=0A= {=0A= local _mp _ret=0A= @@ -62,6 +81,29 @@=0A= fi=0A= }=0A= =0A= +get_opt()=0A= +{=0A= + local _flag _param _opt=0A= +=0A= + _flag=3D$1=0A= + _param=3D$2=0A= +=0A= + _opt=3D${_config##*-${_flag}\ }=0A= + if [ "${_opt}" =3D "${_config}" ]; then=0A= + _opt=3D""=0A= + else=0A= + _opt=3D${_opt%%\ *}=0A= + fi=0A= + eval _${_param}=3D${_opt}=0A= + debug "${_md} ${_param}=3D${_opt}"=0A= +=0A= + if [ -z "${_opt}" ]; then=0A= + return 0=0A= + fi=0A= +=0A= + return 1=0A= +}=0A= +=0A= init_variables()=0A= {=0A= local _i=0A= @@ -75,16 +117,19 @@=0A= eval _perms=3D\$mdconfig_${_md}_perms=0A= eval _files=3D\$mdconfig_${_md}_files=0A= eval _populate=3D\$mdconfig_${_md}_cmd=0A= + eval _zpool=3D\$mdconfig_${_md}_zfs_pool=0A= =0A= - _type=3D${_config##*-t\ }=0A= - _type=3D${_type%%\ *}=0A= + get_opt "t" "type"=0A= + get_opt "f" "file"=0A= if [ -z "${_type}" ]; then=0A= - err 1 "You need to specify \"-t \" in mdconfig_${_md}"=0A= + if [ -n "${_file}" ]; then=0A= + _type=3D"vnode"=0A= + else=0A= + err 1 "You need to specify \"-t \" in mdconfig_${_md}"=0A= + fi=0A= fi=0A= =0A= if [ "${_type}" =3D "vnode" ]; then=0A= - _file=3D${_config##*-f\ }=0A= - _file=3D${_file%%\ *}=0A= if [ -z "${_file}" ]; then=0A= err 2 "You need to specify \"-f \" in mdconfig_${_md} for = vnode devices"=0A= fi=0A= @@ -136,26 +181,42 @@=0A= echo "Creating ${_md} device failed, moving on."=0A= continue=0A= fi=0A= - # Skip fsck for uzip devices.=0A= - if [ "${_file}" !=3D "${_file%.uzip}" ]; then=0A= - _fsck_cmd=3D":"=0A= - elif checkyesno background_fsck; then=0A= - _fsck_cmd=3D"fsck -F"=0A= +=0A= + if [ -n "${_zpool}" ]; then=0A= + if [ "${_type}" =3D "vnode" ]; then=0A= + echo "Importing ZFS pool ${_zpool}."=0A= + zpool import ${_zpool}=0A= + else=0A= + echo "Creating ZFS pool ${_zpool}."=0A= + zpool create ${_zflags} ${_zpool} ${_md}=0A= + fi=0A= else=0A= - _fsck_cmd=3D"fsck"=0A= - fi=0A= - if ! eval ${_fsck_cmd} -p ${_dev} >/dev/null; then=0A= - echo "Fsck failed on ${_dev}, not mounting the filesystem."=0A= - continue=0A= - fi=0A= - if mount -d ${_dev} >/dev/null 2>&1; then=0A= - echo "Mounting ${_dev}."=0A= - mount ${_dev}=0A= + # Skip fsck for uzip devices.=0A= + if [ "${_file}" !=3D "${_file%.uzip}" ]; then=0A= + _fsck_cmd=3D":"=0A= + elif checkyesno background_fsck; then=0A= + _fsck_cmd=3D"fsck -t ufs -F"=0A= + else=0A= + _fsck_cmd=3D"fsck -t ufs"=0A= + fi=0A= + if ! eval ${_fsck_cmd} -p ${_dev} >/dev/null; then=0A= + echo "Fsck failed on ${_dev}, not mounting the filesystem."=0A= + continue=0A= + fi=0A= + if mount -d ${_dev} >/dev/null 2>&1; then=0A= + echo "Mounting ${_dev}."=0A= + mount ${_dev}=0A= + fi=0A= fi=0A= fi=0A= =0A= - for _i in `df ${_dev} 2>/dev/null`; do _mp=3D${_i}; done=0A= - if [ ! -z "${_mp}" -a "${_mp}" =3D "${_mp%%%}" ]; then=0A= + if [ -n "${_zpool}" ]; then=0A= + _mp=3D`zfs list -H -o mountpoint ${_zpool} 2>/dev/null`=0A= + else=0A= + for _i in `df ${_dev} 2>/dev/null`; do _mp=3D${_i}; done=0A= + fi=0A= +=0A= + if [ -n "${_mp}" -a "${_mp}" =3D "${_mp%%%}" ]; then=0A= _mounted=3D"yes"=0A= fi=0A= =0A= ------=_NextPart_000_0570_01CE6545.6E1C99C0--