Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 30 Nov 2013 16:36:18 -0600
From:      Nathan Whitehorn <nwhitehorn@freebsd.org>
To:        Devin Teske <dteske@freebsd.org>
Cc:        "freebsd-arch@freebsd.org" <freebsd-arch@freebsd.org>, Peter Grehan <grehan@freebsd.org>, "Teske, Devin" <Devin.Teske@fisglobal.com>, Current Current <freebsd-current@freebsd.org>, Michael Dexter <editor@callfortesting.org>
Subject:   Re: [CFT] bsdinstall and zfsboot enhancements
Message-ID:  <529A6862.7060308@freebsd.org>
In-Reply-To: <D81082F2-8273-449F-A2EB-DAA12779CAE7@fisglobal.com>
References:  <C9783B1F-20EA-4C08-9947-70DF363E8B6A@fisglobal.com> <5275C597.6070702@freebsd.org> <97944047-D575-4E2E-B687-9871DFE058E3@fisglobal.com> <ABD90FE2-1540-410A-959E-D91D0BE811E3@freebsd.org> <52769CFE.5080707@freebsd.org> <5281340E.8080009@callfortesting.org> <F3512B82-7B2E-40D9-A513-C4C2430F9255@fisglobal.com> <52813E53.20403@freebsd.org> <5281441E.7060806@freebsd.org> <D81082F2-8273-449F-A2EB-DAA12779CAE7@fisglobal.com>

next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.

--Boundary_(ID_ftISHxvL5H+AYDZeUZXaaA)
Content-type: text/plain; CHARSET=US-ASCII
Content-transfer-encoding: 7BIT

On 11/11/13 14:57, Teske, Devin wrote:
> On Nov 11, 2013, at 12:54 PM, Nathan Whitehorn wrote:
>
>> On 11/11/13 14:30, Nathan Whitehorn wrote:
>>> On 11/11/13 14:18, Teske, Devin wrote:
>>>> -----BEGIN PGP SIGNED MESSAGE-----
>>>> Hash: SHA512
>>>>
>>>>
>>>> On Nov 11, 2013, at 11:46 AM, Michael Dexter wrote:
>>>>
>>>>
>>>> Hello all,
>>>>
>>>> I have been experimenting with various BSD and GNU/Linux boot media
>>>> under bhyve and noticed that we may want to accommodate the "LiveCD"
>>>> mode of the installer, which in turn requires the correct console.
>>>>
>>>> Currently, one is prompted for VT100 for installation but this does not
>>>> appear to work/stick for LiveCD mode.
>>>>
>>>> Can anyone verify this?
>>>>
>>>>
>>>> While I developed this patch...
>>>> https://urldefense.proofpoint.com/v1/url?u=http://druidbsd.cvs.sf.net/viewvc/druidbsd/bsdinstall_zfs/usr.sbin%253A%253Absdinstall%253A%253Ascripts%253A%253Aconfig.patch?revision%3D1.10%26view%3Dmarkup&k=%2FbkpAUdJWZuiTILCq%2FFnQg%3D%3D%0A&r=Mrjs6vR4%2Faj2Ns9%2FssHJjg%3D%3D%0A&m=aZg%2BxwqLTX6Zcpf3Rc44iPtAQhFrpqoS4FC5B8ykxJQ%3D%0A&s=6d54d337ea5472f5713ddbe7788d3248c45feefd4b291eab0a976c39e9d40428 
>>>>
>>>> Reasons exist to search for a better solution, see here:
>>>> https://urldefense.proofpoint.com/v1/url?u=http://lists.freebsd.org/pipermail/freebsd-current/2013-November/046148.html&k=%2FbkpAUdJWZuiTILCq%2FFnQg%3D%3D%0A&r=Mrjs6vR4%2Faj2Ns9%2FssHJjg%3D%3D%0A&m=aZg%2BxwqLTX6Zcpf3Rc44iPtAQhFrpqoS4FC5B8ykxJQ%3D%0A&s=5f8128901747346f937ffc4478eadb4bc39059504258dfb9b36f0fb9e7a61b79 
>>>> (and messages that follow it)
>>>>
>>>> Is modifying init(8) still the way to go? What modification do we want to make?
>>>> I'll do the work if we can come to consensus.
>>>>
>>>> Or should we touch up the patch in some way to address the original concerns?
>>>>
>>> I think modifying init is the way to go -- it keeps the install system from interfering with the installed one, as well as fixing this kind of issue with moved hard drives or PXE booting or what have you. If we can provide a guarantee that any system that displays text has a working console (unless explicitly configured not to), useability is improved.
>>>
>>> I would propose one of the following (and volunteer to write the code):
>>>
>>> Option A
>>> ------------
>>>
>>> 1. init checks if there is an entry in /etc/ttys for the terminal[s] corresponding to the value[s] in kern.console
>>> 2. If an entry for each console terminal exists in /etc/ttys, enable it
>>> 3. If not, invent one with a terminal type of "ansi"
>>>
>>> The one issue here is that someone may want to force a particular entry to off and still have it be the kernel console. This is tricky. We could invent a new "status" field that is not "on" or "off" ("auto", maybe, or "ifconsole"?). Which brings us to:
>> One easy way to accomplish this is just to only implement (1) and (3), then comment out the ttyu0 entry in /etc/ttys on x86 instead of marking it "off". Then the behavior is just that a tty marked "off" stays off, one marked "on" stays on, and one not present spawns login with a terminal type corresponding to "console" (by default "unknown") if it happens to be the console. I will implement this over the next few days and then send patches unless anyone has an objection.
> I love it. (smiles)
>
> Excellent idea and that's the winner I think.
>
> +1

This took much longer than I'd anticipated, but the patch to init is
attached. I chose not to make the changes to init rather than
getttyent() and friends in libc, which I am open to revisiting. The
behavior changes are as follows:

If the "console" device in /etc/ttys in marked "on", instead of opening
/dev/console, init will loop through the active kernel console devices,
and for each will:
1. If the kernel console device is in /etc/ttys and marked "on", it
already has a terminal and will be ignored.
2. If marked "off", that is an explicit statement that a console is not
wanted and so it will be ignored.
3. If not present in /etc/ttys, init will run getty with whatever
parameters "console" has.

(3) is the main behavioral change. No changes in behavior will occur if
/etc/ttys is not modified. If we turn on "console" by default, it will
usually have no effect instead of trying to run multiple gettys, which
is new. If we then also comment out the ttyu0 line, instead of marking
it "off", the result will be the conditional presence of a login prompt
on the first serial port depending on whether it is an active console
device for the kernel. I believe this is the behavior we are going for.

Comments and test results would be appreciated.
-Nathan

--Boundary_(ID_ftISHxvL5H+AYDZeUZXaaA)
Content-type: text/plain; CHARSET=US-ASCII; name=init-ttys.diff
Content-transfer-encoding: 7BIT
Content-disposition: attachment; filename=init-ttys.diff

Index: init.c
===================================================================
--- init.c	(revision 258756)
+++ init.c	(working copy)
@@ -1102,22 +1102,84 @@
 }
 
 /*
+ * Do something for all defined TTYs
+ */
+static void
+do_allttys(void (*callback)(struct ttyent *, void *), void *cookie)
+{
+	struct ttyent *typ;
+	char *buf, *cons, *nextcons;
+	size_t len;
+
+	/* Loop through /etc/ttys */
+	while ((typ = getttyent()) != NULL) {
+		if (strcmp(typ->ty_name, "console") == 0)
+			continue;
+
+		callback(typ, cookie);
+	}
+
+	/* Mix in any active console devices not in /etc/ttys */
+	buf = NULL;
+	if (sysctlbyname("kern.console", NULL, &len, NULL, 0) == -1)
+		goto done;
+	buf = malloc(len);
+	if (sysctlbyname("kern.console", buf, &len, NULL, 0) == -1)
+		goto done;
+
+	if ((cons = strchr(buf, '/')) == NULL)
+		goto done;
+	*cons = '\0';
+	nextcons = buf;
+	while ((cons = strsep(&nextcons, ",")) != NULL && strlen(cons) != 0) {
+		typ = getttynam(cons);
+		if (typ != NULL)	/* Skip terminals with known configs */
+			continue;
+		typ = getttynam("console");
+		typ->ty_name = cons;
+
+		callback(typ, cookie);
+	}
+
+done:
+	if (buf != NULL)
+		free(buf);
+	endttyent();
+}
+
+/*
  * Walk the list of ttys and create sessions for each active line.
  */
+struct read_ttys_args {
+	int session_index;
+	session_t *sp;
+};
+
+static void
+read_ttys_callback(struct ttyent *typ, void *cookie)
+{
+	struct read_ttys_args *args = cookie;
+	session_t *snext;
+	
+	if ((snext = new_session(args->sp, ++args->session_index, typ)) != NULL)
+		args->sp = snext;
+}
+
 static state_func_t
 read_ttys(void)
 {
-	int session_index = 0;
-	session_t *sp, *snext;
-	struct ttyent *typ;
+	struct read_ttys_args args;
+	session_t *snext;
 
+	args.session_index = 0;
+
 	/*
 	 * Destroy any previous session state.
 	 * There shouldn't be any, but just in case...
 	 */
-	for (sp = sessions; sp; sp = snext) {
-		snext = sp->se_next;
-		free_session(sp);
+	for (args.sp = sessions; args.sp; args.sp = snext) {
+		snext = args.sp->se_next;
+		free_session(args.sp);
 	}
 	sessions = 0;
 	if (start_session_db())
@@ -1127,12 +1189,8 @@
 	 * Allocate a session entry for each active port.
 	 * Note that sp starts at 0.
 	 */
-	while ((typ = getttyent()) != NULL)
-		if ((snext = new_session(sp, ++session_index, typ)) != NULL)
-			sp = snext;
+	do_allttys(read_ttys_callback, &args);
 
-	endttyent();
-
 	return (state_func_t) multi_user;
 }
 
@@ -1375,85 +1433,93 @@
 /*
  * This is an (n*2)+(n^2) algorithm.  We hope it isn't run often...
  */
-static state_func_t
-clean_ttys(void)
+static void
+clean_ttys_callback(struct ttyent *typ, void *cookie)
 {
+	int *session_index = cookie;
 	session_t *sp, *sprev;
-	struct ttyent *typ;
-	int session_index = 0;
 	int devlen;
 	char *old_getty, *old_window, *old_type;
 
-	/*
-	 * mark all sessions for death, (!SE_PRESENT)
-	 * as we find or create new ones they'll be marked as keepers,
-	 * we'll later nuke all the ones not found in /etc/ttys
-	 */
-	for (sp = sessions; sp != NULL; sp = sp->se_next)
-		sp->se_flags &= ~SE_PRESENT;
-
 	devlen = sizeof(_PATH_DEV) - 1;
-	while ((typ = getttyent()) != NULL) {
-		++session_index;
+	++(*session_index);
 
-		for (sprev = 0, sp = sessions; sp; sprev = sp, sp = sp->se_next)
-			if (strcmp(typ->ty_name, sp->se_device + devlen) == 0)
-				break;
+	for (sprev = 0, sp = sessions; sp; sprev = sp, sp = sp->se_next)
+		if (strcmp(typ->ty_name, sp->se_device + devlen) == 0)
+			break;
 
-		if (sp) {
-			/* we want this one to live */
-			sp->se_flags |= SE_PRESENT;
-			if (sp->se_index != session_index) {
-				warning("port %s changed utmp index from %d to %d",
-				       sp->se_device, sp->se_index,
-				       session_index);
-				sp->se_index = session_index;
-			}
-			if ((typ->ty_status & TTY_ON) == 0 ||
-			    typ->ty_getty == 0) {
-				sp->se_flags |= SE_SHUTDOWN;
-				kill(sp->se_process, SIGHUP);
-				continue;
-			}
-			sp->se_flags &= ~SE_SHUTDOWN;
-			old_getty = sp->se_getty ? strdup(sp->se_getty) : 0;
-			old_window = sp->se_window ? strdup(sp->se_window) : 0;
-			old_type = sp->se_type ? strdup(sp->se_type) : 0;
-			if (setupargv(sp, typ) == 0) {
-				warning("can't parse getty for port %s",
-					sp->se_device);
-				sp->se_flags |= SE_SHUTDOWN;
-				kill(sp->se_process, SIGHUP);
-			}
-			else if (   !old_getty
-				 || (!old_type && sp->se_type)
-				 || (old_type && !sp->se_type)
-				 || (!old_window && sp->se_window)
-				 || (old_window && !sp->se_window)
-				 || (strcmp(old_getty, sp->se_getty) != 0)
-				 || (old_window && strcmp(old_window, sp->se_window) != 0)
-				 || (old_type && strcmp(old_type, sp->se_type) != 0)
-				) {
+	if (sp) {
+		/* we want this one to live */
+		sp->se_flags |= SE_PRESENT;
+		if (sp->se_index != *session_index) {
+			warning("port %s changed utmp index from %d to %d",
+			       sp->se_device, sp->se_index,
+			       *session_index);
+			sp->se_index = *session_index;
+		}
+		if ((typ->ty_status & TTY_ON) == 0 ||
+		    typ->ty_getty == 0) {
+			sp->se_flags |= SE_SHUTDOWN;
+			kill(sp->se_process, SIGHUP);
+			return;
+		}
+		sp->se_flags &= ~SE_SHUTDOWN;
+		old_getty = sp->se_getty ? strdup(sp->se_getty) : 0;
+		old_window = sp->se_window ? strdup(sp->se_window) : 0;
+		old_type = sp->se_type ? strdup(sp->se_type) : 0;
+		if (setupargv(sp, typ) == 0) {
+			warning("can't parse getty for port %s",
+				sp->se_device);
+			sp->se_flags |= SE_SHUTDOWN;
+			kill(sp->se_process, SIGHUP);
+		}
+		else if (   !old_getty
+			 || (!old_type && sp->se_type)
+			 || (old_type && !sp->se_type)
+			 || (!old_window && sp->se_window)
+			 || (old_window && !sp->se_window)
+			 || (strcmp(old_getty, sp->se_getty) != 0)
+			 || (old_window && strcmp(old_window, sp->se_window) != 0)
+			 || (old_type && strcmp(old_type, sp->se_type) != 0)
+			) {
 				/* Don't set SE_SHUTDOWN here */
 				sp->se_nspace = 0;
 				sp->se_started = 0;
 				kill(sp->se_process, SIGHUP);
-			}
-			if (old_getty)
-				free(old_getty);
-			if (old_window)
-				free(old_window);
-			if (old_type)
-				free(old_type);
-			continue;
 		}
-
-		new_session(sprev, session_index, typ);
+		if (old_getty)
+			free(old_getty);
+		if (old_window)
+			free(old_window);
+		if (old_type)
+			free(old_type);
+		return;
 	}
 
-	endttyent();
+	new_session(sprev, *session_index, typ);
+}
 
+static state_func_t
+clean_ttys(void)
+{
+	session_t *sp;
+	int session_index = 0;
+
 	/*
+	 * mark all sessions for death, (!SE_PRESENT)
+	 * as we find or create new ones they'll be marked as keepers,
+	 * we'll later nuke all the ones not found in /etc/ttys
+	 */
+	for (sp = sessions; sp != NULL; sp = sp->se_next)
+		sp->se_flags &= ~SE_PRESENT;
+
+	/*
+	 * Loop over all TTYs and sync state
+	 */
+	do_allttys(clean_ttys_callback, &session_index);
+
+
+	/*
 	 * sweep through and kill all deleted sessions
 	 * ones who's /etc/ttys line was deleted (SE_PRESENT unset)
 	 */

--Boundary_(ID_ftISHxvL5H+AYDZeUZXaaA)--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?529A6862.7060308>