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>