Re: [libvirt] [PATCH] virsh: Allow other escape characters for console
On 11/22/2011 09:18 AM, Michal Privoznik wrote:
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.
---
Okay, this patch is meant as RFC mainly but if it got enough ACKs
I will not hesitate to push it. My biggest concern is the way
of telling virsh customized sequence. I am not big fan of new switch,
however we lack virsh.conf in $conf_dir. Maybe it is the right time
for creating it.
Another approach is to pass the sequence as parameter directly to
'console' command.
I'm leaning 60-40 towards 'virsh -e', since both 'virsh console' and
'virsh start --console' would benefit from shared code, rather than
having to make both of those functions parse in an alternate escape
character.
I agree that a virsh.conf would make it nice to set preferences without
having to always spell it out on the command line, in which case having
virsh.conf serve as an alternate to 'virsh -e ... console' makes more
sense than having it serve as an alternate to 'virsh console -e ...'
(that is, it seems like having virsh.conf override defaults for global
settings makes more sense than having it override defaults of
per-command settings), so that would sway my opinion to 70-30 in favor
of a global -e.
-
-/* ie Ctrl-] as per telnet */
-# define CTRL_CLOSE_BRACKET '\35'
+/*
+ * Convert given character to control character.
+ * Basically, we take lower 5 bits unless given
+ * character is DEL (represented by '?'). Then
+ * we return 127
+ */
+# define CONTROL(c) ((c) == '?' ? ((c) | 0x40) : ((c) & 0x1f))
I've usually seen this written:
CONTROL(c) ((c) ^ 0x40)
This is ASCII-specific, but do we care about EBCDIC?
-int vshRunConsole(virDomainPtr dom, const char *dev_name)
+static char
+vshGetEscapeChar(const char *s)
+{
+ if (*s == '^')
+ return CONTROL(s[1]);
+
+ return *s;
+}
Should we sanity check that the string s is exactly 1 or 2 bytes, that
it is not a 1-byte ^, and that if it is 2 bytes, it starts with ^?
+/* Default escape char Ctrl-] as per telnet */
+#define CTRL_CLOSE_BRACKET "^]"
+
/**
* The log configuration
*/
@@ -249,6 +252,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 {
@@ -796,8 +800,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);
This won't work. You want to undo the CONTROL() conversion, and output
a literal ^ followed by the counterpart character, as in:
vshPrintExtra(ctl, _("Escape character is ^%c\n"), CONTROL(ctl->escapeChar))
assuming that the control character is non-printable, and the
counterpart is printable. Maybe you need a c_isprint() call in there?
@@ -16848,7 +16853,8 @@ vshUsage(void)
" -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"
+ " -V | --version=long version and full
options\n"
+ " -e | --escape <char> set escape sequence for
console\n\n"
Well, we aren't very consistent in that formatting. I'd almost prefer
we change to a single format:
-l | --log=FILE log to FILE
-v short version
-V long version
--version[=TYPE] version, TYPE is short or long (default short)
-e | --escape=CHAR set console escape sequence to CHAR
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)