Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 13 May 2012 19:22:31 -0600
From:      Warner Losh <imp@bsdimp.com>
To:        Jilles Tjoelker <jilles@stack.nl>
Cc:        freebsd-arch@FreeBSD.org
Subject:   Re: [patch] halt/reboot/shutdown cleanup
Message-ID:  <81FA7C45-89FF-475A-A19A-8F761D74483F@bsdimp.com>
In-Reply-To: <20120513220646.GA12826@stack.nl>
References:  <20120513220646.GA12826@stack.nl>

next in thread | previous in thread | raw e-mail | index | archive | help
After the short discussion on IRC, I don't think that there's consensus =
this patch is a good idea at all.  I don't recall that being the outcome =
of the last round of discussions either.

Maybe we need:

#!/bin/sh
case ${HALT_METHOD:-bsd} in
*) /sbin/bsdhalt $*
linux) /sbin/shutdown -h $* now
esac

and similar for reboot, since that would also solve the problem...

Warner

On May 13, 2012, at 4:06 PM, Jilles Tjoelker wrote:

> There is duplication of code between sbin/reboot and init: both can
> cleanly shut down the system, for some value of "cleanly". Because the
> code in init is activated by the kernel (sending a signal to init), it
> makes sense to keep that and remove the duplicate from sbin/reboot. =
This
> also ensures that /etc/rc.shutdown is executed in all "clean" =
shutdowns.
>=20
> Before 9.0, init could not shut down the system 100% correctly during
> single user mode (for example, it inappropriately ran =
/etc/rc.shutdown),
> but this has been fixed.
>=20
> Also, the normal forms of halt and reboot will now call shutdown so
> users get a clear message of the event.
>=20
> Halt and reboot still support the -q option to invoke reboot(2) =
without
> anything else. The -d and -n options now require -q (because init is
> signaled if -q is not used, and init will not do dump or nosync).
>=20
> The -l option of halt and reboot now not only suppresses logging, but
> also user notification. It does this by signaling init directly and =
not
> going through shutdown.
>=20
> The -o option of shutdown goes away because there does not seem any
> point in executing halt or reboot if they are going to send the same
> signal to init anyway.
>=20
> diff --git a/include/paths.h b/include/paths.h
> index 6503934..f93cf39 100644
> --- a/include/paths.h
> +++ b/include/paths.h
> @@ -84,6 +84,7 @@
> #define	_PATH_RSH	"/usr/bin/rsh"
> #define	_PATH_SENDMAIL	"/usr/sbin/sendmail"
> #define	_PATH_SHELLS	"/etc/shells"
> +#define	_PATH_SHUTDOWN	"/sbin/shutdown"
> #define	_PATH_TTY	"/dev/tty"
> #define	_PATH_UNIX	"don't use _PATH_UNIX"
> #define	_PATH_VI	"/usr/bin/vi"
> diff --git a/sbin/reboot/reboot.8 b/sbin/reboot/reboot.8
> index 13d7098..50c27ee 100644
> --- a/sbin/reboot/reboot.8
> +++ b/sbin/reboot/reboot.8
> @@ -55,12 +55,11 @@ The
> .Nm halt
> and
> .Nm
> -utilities flush the file system cache to disk, send all running =
processes
> -a
> -.Dv SIGTERM
> -(and subsequently a
> -.Dv SIGKILL )
> -and, respectively, halt or restart the system.
> +utilities invoke
> +.Xr shutdown 8
> +or signal
> +.Xr init 8
> +to, respectively, halt or restart the system.
> The action is logged, including entering a shutdown record into the =
user
> accounting database.
> .Pp
> @@ -69,7 +68,9 @@ The options are as follows:
> .It Fl d
> The system is requested to create a crash dump.
> This option is
> -supported only when rebooting, and it has no effect unless a dump
> +supported only when rebooting together with the
> +.Fl q
> +option, and it has no effect unless a dump
> device has previously been specified with
> .Xr dumpon 8 .
> .It Fl k Ar kernel
> @@ -86,25 +87,14 @@ This may change in the future.
> .It Fl l
> The halt or reboot is
> .Em not
> -logged to the system log.
> -This option is intended for applications such as
> -.Xr shutdown 8 ,
> -that call
> -.Nm
> -or
> -.Nm halt
> -and log this themselves.
> +logged to the system log and not announced to users.
> .It Fl n
> The file system cache is not flushed.
> -This option should probably not be used.
> +This option should probably not be used and only works together with =
the
> +.Fl q
> +option.
> .It Fl p
> The system will turn off the power if it can.
> -If the power down action fails, the system
> -will halt or reboot normally, depending on whether
> -.Nm halt
> -or
> -.Nm
> -was called.
> .It Fl q
> The system is halted or restarted quickly and ungracefully, and only
> the flushing of the file system cache is performed (if the
> @@ -122,12 +112,6 @@ utilities are nothing more than aliases for the
> and
> .Nm
> utilities.
> -.Pp
> -Normally, the
> -.Xr shutdown 8
> -utility is used when the system needs to be halted or restarted, =
giving
> -users advance warning of their impending doom and cleanly terminating
> -specific programs.
> .Sh SEE ALSO
> .Xr getutxent 3 ,
> .Xr boot 8 ,
> diff --git a/sbin/reboot/reboot.c b/sbin/reboot/reboot.c
> index d927db0..285437a 100644
> --- a/sbin/reboot/reboot.c
> +++ b/sbin/reboot/reboot.c
> @@ -44,31 +44,27 @@ __FBSDID("$FreeBSD$");
> #include <sys/reboot.h>
> #include <sys/time.h>
> #include <sys/types.h>
> -#include <sys/sysctl.h>
> #include <signal.h>
> #include <err.h>
> #include <errno.h>
> #include <fcntl.h>
> +#include <paths.h>
> #include <pwd.h>
> #include <syslog.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> -#include <utmpx.h>
>=20
> static void usage(void);
> -static u_int get_pageins(void);
>=20
> static int dohalt;
>=20
> int
> main(int argc, char *argv[])
> {
> -	struct utmpx utx;
> 	const struct passwd *pw;
> -	int ch, howto, i, fd, lflag, nflag, qflag, sverrno;
> -	u_int pageins;
> +	int ch, howto, fd, lflag, qflag;
> 	const char *user, *kernel =3D NULL;
>=20
> 	if (strcmp(getprogname(), "halt") =3D=3D 0) {
> @@ -76,7 +72,7 @@ main(int argc, char *argv[])
> 		howto =3D RB_HALT;
> 	} else
> 		howto =3D 0;
> -	lflag =3D nflag =3D qflag =3D 0;
> +	lflag =3D qflag =3D 0;
> 	while ((ch =3D getopt(argc, argv, "dk:lnpq")) !=3D -1)
> 		switch(ch) {
> 		case 'd':
> @@ -89,7 +85,6 @@ main(int argc, char *argv[])
> 			lflag =3D 1;
> 			break;
> 		case 'n':
> -			nflag =3D 1;
> 			howto |=3D RB_NOSYNC;
> 			break;
> 		case 'p':
> @@ -107,6 +102,10 @@ main(int argc, char *argv[])
>=20
> 	if ((howto & (RB_DUMP | RB_HALT)) =3D=3D (RB_DUMP | RB_HALT))
> 		errx(1, "cannot dump (-d) when halting; must reboot =
instead");
> +	if (!qflag && (howto & RB_DUMP) !=3D 0)
> +		errx(1, "cannot dump (-d) without immediate reboot(2) =
(-q)");
> +	if (!qflag && (howto & RB_NOSYNC) !=3D 0)
> +		errx(1, "cannot disable sync (-n) without immediate =
reboot(2) (-q)");
> 	if (geteuid()) {
> 		errno =3D EPERM;
> 		err(1, NULL);
> @@ -129,6 +128,16 @@ main(int argc, char *argv[])
> 		}
> 	}
>=20
> +#ifndef RESCUE
> +	if (!lflag) {
> +		execl(_PATH_SHUTDOWN, _PATH_SHUTDOWN,
> +		    (howto & RB_POWEROFF) !=3D 0 ? "-p" :
> +		    (howto & RB_HALT) !=3D 0 ? "-h" :
> +		    "-r", "now", (char *)NULL);
> +		warn("cannot execute %s; signaling init(8)", =
_PATH_SHUTDOWN);
> +	}
> +#endif
> +
> 	/* Log the reboot. */
> 	if (!lflag)  {
> 		if ((user =3D getlogin()) =3D=3D NULL)
> @@ -142,80 +151,12 @@ main(int argc, char *argv[])
> 			syslog(LOG_CRIT, "rebooted by %s", user);
> 		}
> 	}
> -	utx.ut_type =3D SHUTDOWN_TIME;
> -	gettimeofday(&utx.ut_tv, NULL);
> -	pututxline(&utx);
> -
> -	/*
> -	 * Do a sync early on, so disks start transfers while we're off
> -	 * killing processes.  Don't worry about writes done before the
> -	 * processes die, the reboot system call syncs the disks.
> -	 */
> -	if (!nflag)
> -		sync();
> -
> -	/*
> -	 * Ignore signals that we can get as a result of killing
> -	 * parents, group leaders, etc.
> -	 */
> -	(void)signal(SIGHUP,  SIG_IGN);
> -	(void)signal(SIGINT,  SIG_IGN);
> -	(void)signal(SIGQUIT, SIG_IGN);
> -	(void)signal(SIGTERM, SIG_IGN);
> -	(void)signal(SIGTSTP, SIG_IGN);
> -
> -	/*
> -	 * If we're running in a pipeline, we don't want to die
> -	 * after killing whatever we're writing to.
> -	 */
> -	(void)signal(SIGPIPE, SIG_IGN);
> -
> -	/* Just stop init -- if we fail, we'll restart it. */
> -	if (kill(1, SIGTSTP) =3D=3D -1)
> -		err(1, "SIGTSTP init");
> -
> -	/* Send a SIGTERM first, a chance to save the buffers. */
> -	if (kill(-1, SIGTERM) =3D=3D -1 && errno !=3D ESRCH)
> -		err(1, "SIGTERM processes");
> -
> -	/*
> -	 * After the processes receive the signal, start the rest of the
> -	 * buffers on their way.  Wait 5 seconds between the SIGTERM and
> -	 * the SIGKILL to give everybody a chance. If there is a lot of
> -	 * paging activity then wait longer, up to a maximum of approx
> -	 * 60 seconds.
> -	 */
> -	sleep(2);
> -	for (i =3D 0; i < 20; i++) {
> -		pageins =3D get_pageins();
> -		if (!nflag)
> -			sync();
> -		sleep(3);
> -		if (get_pageins() =3D=3D pageins)
> -			break;
> -	}
> -
> -	for (i =3D 1;; ++i) {
> -		if (kill(-1, SIGKILL) =3D=3D -1) {
> -			if (errno =3D=3D ESRCH)
> -				break;
> -			goto restart;
> -		}
> -		if (i > 5) {
> -			(void)fprintf(stderr,
> -			    "WARNING: some process(es) wouldn't die\n");
> -			break;
> -		}
> -		(void)sleep(2 * i);
> -	}
> -
> -	reboot(howto);
> -	/* FALLTHROUGH */
>=20
> -restart:
> -	sverrno =3D errno;
> -	errx(1, "%s%s", kill(1, SIGHUP) =3D=3D -1 ? "(can't restart =
init): " : "",
> -	    strerror(sverrno));
> +	if (kill(1, (howto & RB_POWEROFF) !=3D 0 ? SIGUSR2 :
> +		    (howto & RB_HALT) !=3D 0 ? SIGUSR1 :
> +		    SIGINT) =3D=3D -1)
> +		err(1, "signal init");
> +	pause();
> 	/* NOTREACHED */
> }
>=20
> @@ -228,18 +169,3 @@ usage(void)
> 	    "usage: reboot [-dlnpq] [-k kernel]\n");
> 	exit(1);
> }
> -
> -static u_int
> -get_pageins(void)
> -{
> -	u_int pageins;
> -	size_t len;
> -
> -	len =3D sizeof(pageins);
> -	if (sysctlbyname("vm.stats.vm.v_swappgsin", &pageins, &len, =
NULL, 0)
> -	    !=3D 0) {
> -		warnx("v_swappgsin");
> -		return (0);
> -	}
> -	return pageins;
> -}
> diff --git a/sbin/shutdown/shutdown.8 b/sbin/shutdown/shutdown.8
> index b7067e1..37152ac 100644
> --- a/sbin/shutdown/shutdown.8
> +++ b/sbin/shutdown/shutdown.8
> @@ -42,10 +42,6 @@
> .Fl h | Fl p |
> .Fl r | Fl k
> .Oc
> -.Oo
> -.Fl o
> -.Op Fl n
> -.Oc
> .Ar time
> .Op Ar warning-message ...
> .Nm poweroff
> @@ -77,30 +73,6 @@ The
> option
> does not actually halt the system, but leaves the
> system multi-user with logins disabled (for all but super-user).
> -.It Fl o
> -If one of the
> -.Fl h ,
> -.Fl p
> -or
> -.Fl r
> -options are specified,
> -.Nm
> -will execute
> -.Xr halt 8
> -or
> -.Xr reboot 8
> -instead of sending a signal to
> -.Xr init 8 .
> -.It Fl n
> -If the
> -.Fl o
> -option is specified, prevent the file system cache from being flushed =
by passing
> -.Fl n
> -to
> -.Xr halt 8
> -or
> -.Xr reboot 8 .
> -This option should probably not be used.
> .It Ar time
> .Ar Time
> is the time at which
> diff --git a/sbin/shutdown/shutdown.c b/sbin/shutdown/shutdown.c
> index 6e662a8..945bef2 100644
> --- a/sbin/shutdown/shutdown.c
> +++ b/sbin/shutdown/shutdown.c
> @@ -88,9 +88,9 @@ static struct interval {
> #undef S
>=20
> static time_t offset, shuttime;
> -static int dohalt, dopower, doreboot, killflg, mbuflen, oflag;
> +static int dohalt, dopower, doreboot, killflg, mbuflen;
> static char mbuf[BUFSIZ];
> -static const char *nosync, *whom;
> +static const char *whom;
>=20
> static void badtime(void);
> static void perform_shutdown(void);
> @@ -116,7 +116,6 @@ main(int argc, char **argv)
> 		errx(1, "NOT super-user");
> #endif
>=20
> -	nosync =3D NULL;
> 	readstdin =3D 0;
>=20
> 	/*
> @@ -152,10 +151,8 @@ main(int argc, char **argv)
> 			killflg =3D 1;
> 			break;
> 		case 'n':
> -			nosync =3D "-n";
> -			break;
> 		case 'o':
> -			oflag =3D 1;
> +			/* Ignore -n and -o for compatibility. */
> 			break;
> 		case 'p':
> 			dopower =3D 1;
> @@ -176,12 +173,6 @@ main(int argc, char **argv)
> 	if (killflg + doreboot + dohalt + dopower > 1)
> 		usage("incompatible switches -h, -k, -p and -r");
>=20
> -	if (oflag && !(dohalt || dopower || doreboot))
> -		usage("-o requires -h, -p or -r");
> -
> -	if (nosync !=3D NULL && !oflag)
> -		usage("-n requires -o");
> -
> 	getoffset(*argv++);
>=20
> poweroff:
> @@ -351,8 +342,6 @@ timeout(int signo __unused)
> static void
> perform_shutdown(void)
> {
> -	char *empty_environ[] =3D { NULL };
> -
> 	syslog(LOG_NOTICE, "%s by %s: %s",
> 	    doreboot ? "reboot" : dohalt ? "halt" : dopower ? =
"power-down" :=20
> 	    "shutdown", whom, mbuf);
> @@ -370,39 +359,12 @@ perform_shutdown(void)
> 		(void)printf("halt");
> 	else if (dopower)
> 		(void)printf("power-down");
> -	if (nosync !=3D NULL)
> -		(void)printf(" no sync");
> 	(void)printf("\nkill -HUP 1\n");
> #else
> -	if (!oflag) {
> -		(void)kill(1, doreboot ? SIGINT :	/* reboot */
> -			      dohalt ? SIGUSR1 :	/* halt */
> -			      dopower ? SIGUSR2 :	/* power-down */
> -			      SIGTERM);			/* single-user =
*/
> -	} else {
> -		if (doreboot) {
> -			execle(_PATH_REBOOT, "reboot", "-l", nosync,=20
> -				(char *)NULL, empty_environ);
> -			syslog(LOG_ERR, "shutdown: can't exec %s: %m.",
> -				_PATH_REBOOT);
> -			warn(_PATH_REBOOT);
> -		}
> -		else if (dohalt) {
> -			execle(_PATH_HALT, "halt", "-l", nosync,
> -				(char *)NULL, empty_environ);
> -			syslog(LOG_ERR, "shutdown: can't exec %s: %m.",
> -				_PATH_HALT);
> -			warn(_PATH_HALT);
> -		}
> -		else if (dopower) {
> -			execle(_PATH_HALT, "halt", "-l", "-p", nosync,
> -				(char *)NULL, empty_environ);
> -			syslog(LOG_ERR, "shutdown: can't exec %s: %m.",
> -				_PATH_HALT);
> -			warn(_PATH_HALT);
> -		}
> -		(void)kill(1, SIGTERM);		/* to single-user */
> -	}
> +	(void)kill(1, doreboot ? SIGINT :	/* reboot */
> +		      dohalt ? SIGUSR1 :	/* halt */
> +		      dopower ? SIGUSR2 :	/* power-down */
> +		      SIGTERM);			/* single-user */
> #endif
> 	finish(0);
> }
> @@ -534,7 +496,7 @@ usage(const char *cp)
> 	if (cp !=3D NULL)
> 		warnx("%s", cp);
> 	(void)fprintf(stderr,
> -	    "usage: shutdown [-] [-h | -p | -r | -k] [-o [-n]] time =
[warning-message ...]\n"
> +	    "usage: shutdown [-] [-h | -p | -r | -k] time =
[warning-message ...]\n"
> 	    "       poweroff\n");
> 	exit(1);
> }
>=20
> --=20
> Jilles Tjoelker
> _______________________________________________
> freebsd-arch@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to =
"freebsd-arch-unsubscribe@freebsd.org"
>=20
>=20




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?81FA7C45-89FF-475A-A19A-8F761D74483F>