Re: rc.subr: unset _rc_pid (fixes network after acpi S3 wakeup)

[christos%astron.com@localhost (Christos Zoulas)]

In article <58CEA448.2050405%eq.cz@localhost>, rudolf  <netbsd%eq.cz@localhost> wrote:
>-=-=-=-=-=-
>
>Hi,
>
>I've recently switched from netbsd-6 to netbsd-7 (amd64) and today i 
>found out that the wakeup from the S3 acpi sleep state on my old 
>ThinkPad SL510 finally reliably works (using hw.acpi.sleep.vbios=2). 
>Thanks to everybody responsible!

Great.

>
>One minor problem was the network connection, which presented itself by 
>a missing default route. I am using a statically configured ip address 
>on a re0 interface.
>
>The powerd script /etc/powerd/scripts/sleep_button is responsible for 
>invoking "/etc/rc.d/network start". Setting the shell xtrace in 
>/etc/rc.d/network revealed (see attachment last_xtrace_lines.txt and 
>_have_rc_postprocessor() from rc.subr) that the network script is 
>failing because it thinks (due to the inherited fd 9 and environment 
>variables: /etc/rc{,.subr} => powerd => /etc/powerd/scripts/sleep_button 
>=> /etc/rc.d/network) it is running from /etc/rc and writes to closed fd 
>9 (_rc_postprocessor_fd). One way to avoid this is to unset the 
>variables exported by /etc/rc and /etc/rc.subr before running rc.d commands.

Thanks for tracking that down. For current (comments inline):

>
>--- etc/rc.subr.orig	2017-03-19 15:04:05.000000000 +0100
>+++ etc/rc.subr	2017-03-19 15:15:36.000000000 +0100
>@@ -663,18 +663,22 @@
> 					# setup the command to run, and run it
> 					#
> 			echo "Starting ${name}."
>+			exported_rc_vars="RC_PID _rc_pid \
>+_rc_original_stdout_fd _rc_original_stderr_fd _rc_postprocessor_fd"
> 			if [ -n "$_chroot" ]; then
>-				_doit="\
>+				_doit="(\
>+unset $exported_rc_vars; \
> ${_env:+env $_env }\
> ${_nice:+nice -n $_nice }\
> chroot ${_user:+-u $_user }${_group:+-g $_group }${_groups:+-G $_groups }\
>-$_chroot $command $rc_flags $command_args"
>+$_chroot $command $rc_flags $command_args)"
> 			else
>-				_doit="\
>+				_doit="(\
>+unset $exported_rc_vars; \
> ${_chdir:+cd $_chdir; }\
> ${_env:+env $_env }\
> ${_nice:+nice -n $_nice }\
>-$command $rc_flags $command_args"
>+$command $rc_flags $command_args)"
> 				if [ -n "$_user" ]; then
> 				    _doit="su -m $_user -c 'sh -c \"$_doit\"'"
> 				fi

- why kill those only for 'start'? I guess it is only important for start,
  but...
- I don't think think that the parens are needed; after all the cd command
  is separate.
- Why not append those to $_env as "RC_PID= _rc_pid=" ...

>@@ -818,6 +822,7 @@
> 	[ -n "${_rc_postprocessor_fd}" ] || { unset _rc_pid; return 1; }
> 	eval ": >&${_rc_postprocessor_fd}" 2>/dev/null \
> 	|| { unset _rc_pid; return 1; }
>+	fdflags -s +cloexec "${_rc_postprocessor_fd}"

I think this belongs to rc, where the fd is created and I've committed
the fdflags line there.

Thanks,

christos