On 01/16/2017 08:27 PM, Jan Kiszka wrote: > On 2017-01-16 17:07, Ralf Ramsauer wrote: >> Similar to other tools, like 'tail -f', allow 'jailhosue enable' to >> follow the console output when jailhouse gets enabled. >> >> On failure, we will print the console as well (if available). >> >> 'jailhouse enable CELL -v' will print the hypervisor's console content >> upon enabling jailhouse and terminate. > > I wonder if we should have some "jailhouse console" as well or instead > of enable -f/-v. Would allow to attach and print the content even after > a non-verbose enable. > > OK, let's keep it simple: separate "jailhouse console [-f | --follow]", > callable at any time (provided the driver exists). Ok, but still dump the console content if enabling the hypervisor fails? > >> >> Signed-off-by: Ralf Ramsauer <[email protected]> >> --- >> tools/jailhouse.c | 64 >> +++++++++++++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 57 insertions(+), 7 deletions(-) >> >> diff --git a/tools/jailhouse.c b/tools/jailhouse.c >> index ab495fda..5760b087 100644 >> --- a/tools/jailhouse.c >> +++ b/tools/jailhouse.c >> @@ -62,7 +62,8 @@ static void __attribute__((noreturn)) help(char *prog, int >> exit_status) >> >> printf("Usage: %s { COMMAND | --help || --version }\n" > > (hmm, looks like the is one "|" too much after --help) let me pick this up and squash it to this commit. > >> "\nAvailable commands:\n" >> - " enable SYSCONFIG\n" >> + " enable SYSCONFIG" >> + " { [ -f | --follow ] | [ -v | --verbose] }\n" > > No unneeded line-wrap, please. Oh, it's exactly 80 characters! Must have happened during writing. The other day, it was longer :) > >> " disable\n" >> " cell create CELLCONFIG\n" >> " cell list\n" >> @@ -86,6 +87,21 @@ static bool match_opt(const char *argv, const char >> *short_opt, >> strcmp(argv, long_opt) == 0; >> } >> >> +static int fd_set_nonblock(int fd) >> +{ >> + int ret; >> + >> + ret = fcntl(fd, F_GETFL, 0); >> + if (ret == -1) >> + return -errno; >> + >> + ret |= O_NONBLOCK; >> + ret = fcntl(fd, F_SETFL, ret); >> + if (ret == -1) >> + return -errno; >> + return 0; >> +} >> + >> static void call_extension_script(const char *cmd, int argc, char *argv[]) >> { >> const struct extension *ext; >> @@ -207,21 +223,55 @@ static int enable(int argc, char *argv[]) >> { >> void *config; >> int err, fd; >> + char console_buffer[128]; >> + ssize_t r; >> + bool dump_console = false; >> >> - if (argc != 3) >> + fd = open_dev(); >> + >> + if (argc < 3 || argc > 4) >> help(argv[0], 1); >> >> - config = read_file(argv[2], NULL); >> + if (argc == 4 && match_opt(argv[3], "-f", "--follow")) >> + dump_console = true; >> >> - fd = open_dev(); >> + if (argc == 4 && match_opt(argv[3], "-v", "--verbose")) { >> + dump_console = true; >> + err = fd_set_nonblock(fd); >> + if (err) { >> + perror("FD_SET_NONBLOCK"); >> + goto fd_close; >> + } >> + } >> + >> + config = read_file(argv[2], NULL); >> >> err = ioctl(fd, JAILHOUSE_ENABLE, config); >> - if (err) >> + if (err) { >> perror("JAILHOUSE_ENABLE"); >> + err = fd_set_nonblock(fd); >> + if (err) { >> + perror("FD_SET_NONBLOCK"); >> + goto config_free; >> + } >> + dump_console = true; >> + } >> >> - close(fd); >> - free(config); >> + if (dump_console) { >> + do { >> + r = read(fd, console_buffer, sizeof(console_buffer)); >> + if (r < 0) >> + break; >> + r = write(STDOUT_FILENO, console_buffer, r); >> + } while(r); > > while (r) You mean replacing the do {} while() with a while() {}?
Then I would have to set r first. Thanks for comments Ralf > >> + if (r < 0) >> + err = r; >> + } >> >> +config_free: >> + free(config); >> +fd_close: >> + close(fd); >> return err; >> } >> >> > > Jan > -- You received this message because you are subscribed to the Google Groups "Jailhouse" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
