Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CODINGSTYLE.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ coding style should follow the

You may use tools like
[uncrustify](http://uncrustify.sourceforge.net/) with this
[config file](https://github.com/freebsd/pkg/blob/master/freebsd.cfg)
[config file](https://raw.githubusercontent.com/freebsd/pkg/master/freebsd.cfg)
for *new* code, though the result may not be perfect.

Keep in mind that, especially for most of the `bhyve` derived code, it
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ HYPERKIT_LIB_SRC := \
src/lib/fwctl.c \
src/lib/inout.c \
src/lib/ioapic.c \
src/lib/log.c \
src/lib/md5c.c \
src/lib/mem.c \
src/lib/mevent.c \
Expand Down
68 changes: 44 additions & 24 deletions go/hyperkit.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ const (
ConsoleStdio = iota
// ConsoleFile configures console to a tty and output to a file
ConsoleFile
// ConsoleLog configures console to a tty and sends its contents to the logs
ConsoleLog

defaultVPNKitSock = "Library/Containers/com.docker.docker/Data/s50"

Expand Down Expand Up @@ -117,8 +119,7 @@ type HyperKit struct {
// Memory is the amount of megabytes of memory for the VM.
Memory int `json:"memory"`

// Console defines where the console of the VM should be
// connected to. ConsoleStdio and ConsoleFile are supported.
// Console defines where the console of the VM should be connected to.
Console int `json:"console"`

// Below here are internal members, but they are exported so
Expand Down Expand Up @@ -199,11 +200,15 @@ func (h *HyperKit) check() error {
log.Debugf("hyperkit: check %#v", h)
var err error
// Sanity checks on configuration
if h.Console == ConsoleFile && h.StateDir == "" {
return fmt.Errorf("If ConsoleFile is set, StateDir must be specified")
}
if h.Console == ConsoleStdio && !isTerminal(os.Stdout) && h.StateDir == "" {
return fmt.Errorf("If ConsoleStdio is set but stdio is not a terminal, StateDir must be specified")
switch h.Console {
case ConsoleFile, ConsoleLog:
if h.StateDir == "" {
return fmt.Errorf("If ConsoleFile or ConsoleLog is set, StateDir must be specified")
}
case ConsoleStdio:
if !isTerminal(os.Stdout) && h.StateDir == "" {
return fmt.Errorf("If ConsoleStdio is set but stdio is not a terminal, StateDir must be specified")
}
}
for _, image := range h.ISOImages {
if _, err = os.Stat(image); os.IsNotExist(err) {
Expand Down Expand Up @@ -420,10 +425,20 @@ func (h *HyperKit) buildArgs(cmdline string) {
nextSlot++
}

if h.Console == ConsoleStdio && isTerminal(os.Stdout) {
a = append(a, "-l", "com1,stdio")
} else if h.StateDir != "" {
a = append(a, "-l", fmt.Sprintf("com1,autopty=%s/tty,log=%s/console-ring", h.StateDir, h.StateDir))
// -l: LPC device configuration.
{
cfg := "com1"
if h.Console == ConsoleStdio && isTerminal(os.Stdout) {
cfg += fmt.Sprintf(",stdio")
} else {
cfg += fmt.Sprintf(",autopty=%s/tty", h.StateDir)
}
if h.Console == ConsoleLog {
cfg += fmt.Sprintf(",asl")
} else {
cfg += fmt.Sprintf(",log=%s/console-ring", h.StateDir)
}
a = append(a, "-l", cfg)
}

if h.Bootrom == "" {
Expand All @@ -440,6 +455,22 @@ func (h *HyperKit) buildArgs(cmdline string) {
log.Debugf("hyperkit: CmdLine: %#v", h.CmdLine)
}

// openTTY opens the tty files for reading, and returns it.
func (h *HyperKit) openTTY() *os.File {
path := fmt.Sprintf("%s/tty", h.StateDir)
for {
if res, err := os.OpenFile(path, os.O_RDONLY, 0); err != nil {
log.Infof("hyperkit: openTTY: %v, retrying", err)
time.Sleep(10 * time.Millisecond)
} else {
log.Infof("hyperkit: openTTY: got %v", path)
saneTerminal(res)
setRaw(res)
return res
}
}
}

// execute forges the command to run hyperkit, runs and returns it.
// It also plumbs stdin/stdout/stderr.
func (h *HyperKit) execute() (*exec.Cmd, error) {
Expand Down Expand Up @@ -469,20 +500,9 @@ func (h *HyperKit) execute() (*exec.Cmd, error) {
cmd.Stderr = os.Stderr
} else {
go func() {
ttyPath := fmt.Sprintf("%s/tty", h.StateDir)
var tty *os.File
for {
var err error
tty, err = os.OpenFile(ttyPath, os.O_RDONLY, 0)
if err == nil {
break
}
time.Sleep(10 * time.Millisecond)
}
saneTerminal(tty)
setRaw(tty)
tty := h.openTTY()
defer tty.Close()
io.Copy(os.Stdout, tty)
tty.Close()
}()
}
} else if log != nil {
Expand Down
7 changes: 7 additions & 0 deletions src/include/xhyve/log.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#pragma once

/* Initialize ASL logger and local buffer. */
void log_init(void);

/* Send one character to the logger: wait for full lines before actually sending. */
void log_put(uint8_t _c);
50 changes: 50 additions & 0 deletions src/lib/log.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
#include <asl.h>
#include <pwd.h>
#include <fcntl.h>
#include <stdio.h>
#include <time.h>

#include <SystemConfiguration/SystemConfiguration.h>

#include <xhyve/log.h>

static aslclient log_client = NULL;
static aslmsg log_msg = NULL;

static unsigned char buf[4096];
/* Index of the _next_ character to insert in the buffer. */
static size_t buf_idx = 0;

/* asl is deprecated in favor of os_log starting with macOS 10.12. */
#pragma GCC diagnostic ignored "-Wdeprecated-declarations"

/* Initialize ASL logger and local buffer. */
void log_init(void)
{
log_client = asl_open(NULL, NULL, 0);
log_msg = asl_new(ASL_TYPE_MSG);
}


/* Send the content of the buffer to the logger. */
static void log_flush(void)
{
buf[buf_idx] = 0;
asl_log(log_client, log_msg, ASL_LEVEL_NOTICE, "%s", buf);
buf_idx = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care that \n\n\n\n will log a bunch of blank messages? A simple if (buf_idx == 0) return check at the top would fix that if we did.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, that's what I wanted, to keep the current output.

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, that's a good point.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, would you happen to know why we have two whales? Maybe it's D4D specific, I have not checked yet, but I'm kinda worried by the backslashes having been processed.

image

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was some strangeness wrt how/where motd was handled in linuxkit but that was more than a year ago, I don't recall having seen anything like this recently.

Are you running the getty in a container or within the root? Or even both (which might explain things)?

I presume the console is functional (i.e. you can login and type normally)? If it isn't that might suggest you've got two getty processes somehow.

Only other thought I have is if you had configured two serial ports you would get a getty and motd on both of them, but the patches here would combine them into the logs without indicating which was which. If that were the case I'd expect them to be more mixed up though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before these patches, both whales were already in console-ring. I have not dug the issue yet, so I can't even answer to your questions now. It was just in case it rang a bell. Thanks for the hints, I'll use them when I focus on the twin-whale-issue.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually looks like I misremembered. In LinuxKit (modern ones at least) it is actually an initscript which cats /etc/issue (not motd) to any device with a console= on the kernel command line.

Looks like agetty (which LinuxKit uses) will also print /etc/issue but if it is in a container then that is a different /etc/issue, unless you bind it in or arrange to use a different getty container. LinuxKit demos don't have that bind but maybe you do? Or if you were running getty on the host you would see it.

}


/* Send one character to the logger: wait for full lines before actually sending. */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a slight risk that hyperkit or the guest might crash with something useful still buffered.

There isn't much we can sensibly do if hyperkit has crashed, but a guest crash will look like a VM shutdown and be controlled from the hypervisor PoV -- perhaps (as a future improvement?) we should add a log_flush somewhere on the hyperkit exit path?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good.

void log_put(uint8_t c)
{
if ((c == '\n') || (c == 0)) {
log_flush();
} else {
if (buf_idx + 2 >= sizeof(buf)) {
log_flush();
}
buf[buf_idx] = c;
++buf_idx;
}
}
34 changes: 21 additions & 13 deletions src/lib/uart_emul.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,14 @@
#include <assert.h>
#include <errno.h>
#include <sys/mman.h>
#include <xhyve/log.h>
#include <xhyve/support/ns16550.h>
#include <xhyve/mevent.h>
#include <xhyve/uart_emul.h>

#define COM1_BASE 0x3F8
#define COM1_BASE 0x3F8
#define COM1_IRQ 4
#define COM2_BASE 0x2F8
#define COM2_BASE 0x2F8
#define COM2_IRQ 3

#define DEFAULT_RCLK 1843200
Expand Down Expand Up @@ -91,7 +92,7 @@ struct fifo {
struct ttyfd {
bool opened;
int fd; /* tty device file descriptor */
int sfd;
int sfd;
char *name; /* slave pty name when using autopty*/
struct termios tio_orig, tio_new; /* I/O Terminals */
};
Expand Down Expand Up @@ -121,6 +122,7 @@ struct uart_softc {

struct ttyfd tty;
struct log log;
bool asl; /* Output to Apple logger. */
bool thre_int_pending; /* THRE interrupt pending */

void *arg;
Expand Down Expand Up @@ -427,7 +429,7 @@ uart_write(struct uart_softc *sc, int offset, uint8_t value)
}
}

switch (offset) {
switch (offset) {
case REG_DATA:
if (sc->mcr & MCR_LOOPBACK) {
if (rxfifo_putchar(sc, value) != 0)
Expand All @@ -436,6 +438,8 @@ uart_write(struct uart_softc *sc, int offset, uint8_t value)
ttywrite(&sc->tty, value);
if (sc->log.ring)
ringwrite(&sc->log, value);
if (sc->asl)
log_put(value);
} /* else drop on floor */
sc->thre_int_pending = true;
break;
Expand Down Expand Up @@ -681,15 +685,15 @@ uart_tty_backend(struct uart_softc *sc, const char *backend)
static char *
copy_up_to_comma(const char *from)
{
char *comma = strchr(from, ',');
char *tmp = NULL;
if (comma == NULL) {
tmp = strdup(from); /* rest of string */
} else {
ptrdiff_t length = comma - from;
tmp = strndup(from, (size_t)length);
}
return tmp;
char *comma = strchr(from, ',');
char *tmp = NULL;
if (comma == NULL) {
tmp = strdup(from); /* rest of string */
} else {
ptrdiff_t length = comma - from;
tmp = strndup(from, (size_t)length);
}
return tmp;
}

int
Expand Down Expand Up @@ -773,6 +777,10 @@ uart_set_backend(struct uart_softc *sc, const char *backend, const char *devname
if (uart_mapring(sc, logname) == -1) {
goto err;
}
} else if (strcmp("asl", backend) == 0) {
sc->asl = true;
log_init();
retval = 0;
} else if (uart_tty_backend(sc, backend) == 0) {
retval = 0;
} else {
Expand Down