Currently virsh supports only ^] as escape character for console. However, some users might want to use something else. This patch creates such ability by specifying '-e' switch on virsh command line. --- diff to v1: -Eric's review included tools/console.c | 26 +++++++++++++++++++++----- tools/console.h | 4 +++- tools/virsh.c | 40 +++++++++++++++++++++++++++++++--------- tools/virsh.pod | 5 +++++ 4 files changed, 60 insertions(+), 15 deletions(-) diff --git a/tools/console.c b/tools/console.c index 0f85bc7..060629f 100644 --- a/tools/console.c +++ b/tools/console.c @@ -43,9 +43,11 @@ # include "memory.h" # include "virterror_internal.h" - -/* ie Ctrl-] as per telnet */ -# define CTRL_CLOSE_BRACKET '\35' +/* + * Convert given character to control character. + * Basically, we take lower 6 bits. + */ +# define CONTROL(c) ((c) ^ 0x40) # define VIR_FROM_THIS VIR_FROM_NONE @@ -66,6 +68,8 @@ struct virConsole { struct virConsoleBuffer streamToTerminal; struct virConsoleBuffer terminalToStream; + + char escapeChar; }; static int got_signal = 0; @@ -215,7 +219,7 @@ virConsoleEventOnStdin(int watch ATTRIBUTE_UNUSED, virConsoleShutdown(con); return; } - if (con->terminalToStream.data[con->terminalToStream.offset] == CTRL_CLOSE_BRACKET) { + if (con->terminalToStream.data[con->terminalToStream.offset] == con->escapeChar) { virConsoleShutdown(con); return; } @@ -278,7 +282,18 @@ virConsoleEventOnStdout(int watch ATTRIBUTE_UNUSED, } -int vshRunConsole(virDomainPtr dom, const char *dev_name) +static char +vshGetEscapeChar(const char *s) +{ + if (*s == '^') + return CONTROL(s[1]); + + return *s; +} + +int vshRunConsole(virDomainPtr dom, + const char *dev_name, + const char *escape_seq) { int ret = -1; struct termios ttyattr, rawattr; @@ -326,6 +341,7 @@ int vshRunConsole(virDomainPtr dom, const char *dev_name) goto cleanup; } + con->escapeChar = vshGetEscapeChar(escape_seq); con->st = virStreamNew(virDomainGetConnect(dom), VIR_STREAM_NONBLOCK); if (!con->st) diff --git a/tools/console.h b/tools/console.h index 9b05ff1..8cca08f 100644 --- a/tools/console.h +++ b/tools/console.h @@ -25,7 +25,9 @@ # ifndef WIN32 -int vshRunConsole(virDomainPtr dom, const char *dev_name); +int vshRunConsole(virDomainPtr dom, + const char *dev_name, + const char *escape_seq); # endif /* !WIN32 */ diff --git a/tools/virsh.c b/tools/virsh.c index 50fc98d..28e8d45 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -76,6 +76,9 @@ static char *progname; ((((int) ((T)->tv_sec - (U)->tv_sec)) * 1000000.0 + \ ((int) ((T)->tv_usec - (U)->tv_usec))) / 1000.0) +/* Default escape char Ctrl-] as per telnet */ +#define CTRL_CLOSE_BRACKET "^]" + /** * The log configuration */ @@ -250,6 +253,7 @@ typedef struct __vshControl { virDomainGetState is not supported */ bool useSnapshotOld; /* cannot use virDomainSnapshotGetParent or virDomainSnapshotNumChildren */ + const char *escapeChar; /* Escape character for domain console */ } __vshControl; typedef struct vshCmdGrp { @@ -797,8 +801,8 @@ cmdRunConsole(vshControl *ctl, virDomainPtr dom, const char *name) } vshPrintExtra(ctl, _("Connected to domain %s\n"), virDomainGetName(dom)); - vshPrintExtra(ctl, "%s", _("Escape character is ^]\n")); - if (vshRunConsole(dom, name) == 0) + vshPrintExtra(ctl, _("Escape character is %s\n"), ctl->escapeChar); + if (vshRunConsole(dom, name, ctl->escapeChar) == 0) ret = true; cleanup: @@ -17072,6 +17076,7 @@ vshDeinit(vshControl *ctl) vshReadlineDeinit(ctl); vshCloseLogFile(ctl); VIR_FREE(ctl->name); + VIR_FREE(ctl->escapeChar); if (ctl->conn) { int ret; if ((ret = virConnectClose(ctl->conn)) != 0) { @@ -17095,15 +17100,17 @@ vshUsage(void) fprintf(stdout, _("\n%s [options]... [<command_string>]" "\n%s [options]... <command> [args...]\n\n" " options:\n" - " -c | --connect <uri> hypervisor connection URI\n" + " -c | --connect=URI hypervisor connection URI\n" " -r | --readonly connect readonly\n" - " -d | --debug <num> debug level [0-4]\n" + " -d | --debug=num debug level [0-4]\n" " -h | --help this help\n" " -q | --quiet quiet mode\n" " -t | --timing print timing information\n" - " -l | --log <file> output logging to file\n" - " -v | --version[=short] program version\n" - " -V | --version=long version and full options\n\n" + " -l | --log=FILE output logging to file\n" + " -v short version\n" + " -V long version\n" + " --version[=TYPE] version, TYPE is short or long (default short)\n" + " -e | --escape <char> set escape sequence for console\n\n" " commands (non interactive mode):\n\n"), progname, progname); for (grp = cmdGroups; grp->name; grp++) { @@ -17254,7 +17261,7 @@ static bool vshParseArgv(vshControl *ctl, int argc, char **argv) { bool help = false; - int arg; + int arg, len; struct option opt[] = { {"debug", required_argument, NULL, 'd'}, {"help", no_argument, NULL, 'h'}, @@ -17264,13 +17271,14 @@ vshParseArgv(vshControl *ctl, int argc, char **argv) {"connect", required_argument, NULL, 'c'}, {"readonly", no_argument, NULL, 'r'}, {"log", required_argument, NULL, 'l'}, + {"escape", required_argument, NULL, 'e'}, {NULL, 0, NULL, 0} }; /* Standard (non-command) options. The leading + ensures that no * argument reordering takes place, so that command options are * not confused with top-level virsh options. */ - while ((arg = getopt_long(argc, argv, "+d:hqtc:vVrl:", opt, NULL)) != -1) { + while ((arg = getopt_long(argc, argv, "+d:hqtc:vVrl:e:", opt, NULL)) != -1) { switch (arg) { case 'd': if (virStrToLong_i(optarg, NULL, 10, &ctl->debug) < 0) { @@ -17305,6 +17313,18 @@ vshParseArgv(vshControl *ctl, int argc, char **argv) case 'l': ctl->logfile = vshStrdup(ctl, optarg); break; + case 'e': + len = strlen(optarg); + + if ((len == 2 && *optarg == '^') || + (len == 1 && *optarg != '^')) { + ctl->escapeChar = vshStrdup(ctl, optarg); + } else { + vshError(ctl, _("Invalid string '%s' for escape sequence"), + optarg); + exit(EXIT_FAILURE); + } + break; default: vshError(ctl, _("unsupported option '-%c'. See --help."), arg); exit(EXIT_FAILURE); @@ -17346,6 +17366,8 @@ main(int argc, char **argv) ctl->imode = true; /* default is interactive mode */ ctl->log_fd = -1; /* Initialize log file descriptor */ ctl->debug = VSH_DEBUG_DEFAULT; + ctl->escapeChar = vshStrdup(ctl, CTRL_CLOSE_BRACKET); + if (!setlocale(LC_ALL, "")) { perror("setlocale"); diff --git a/tools/virsh.pod b/tools/virsh.pod index db872dd..08b761d 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -92,6 +92,11 @@ option of the B<connect> command. Output elapsed time information for each command. +=item B<-e>, B<--escape> I<string> + +Set alternative escape sequence for I<console> command. By default, +telnet's ^] is used. + =back =head1 NOTES -- 1.7.3.4

On 11/24/2011 04:03 AM, Michal Privoznik wrote: Maybe tweak the comment with: s/we take/we assume ASCII, and take/ This confused me in v1. Here, you are storing the string representation of the escape char, not the escape char itself. How about tweaking the comment: /* String representation of console escape character */ Here, I'd write --debug=NUM, to make it obvious that NUM is a metasyntax to be replaced by a number. Mem leak - this overwrites malloc'd memory... from here. But why do you need vshStrdup in the first place? Why not just directly assign ctl->escapeChar = CTRL_CLOSE_BRACKET here, and ctl->escapeChar = optarg above? It might look better in the man page to use B<^]> for emphasis. ACK with nits fixed, and apologies for the slow review. -- Eric Blake eblake(a)redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org