From cce1998a645333e61d6629a0ef2024448c7ec383 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Tue, 8 Dec 2009 18:05:02 +0000 Subject: [PATCH] Make QEMU text monitor parsing more robust The QEMU 0.10.0 release (and possibly other 0.10.x) has a bug where it sometimes/often forgets to display the initial monitor greeting line, soley printing a (qemu). This in turn confuses the text console parsing because it has a '(qemu)' it is not expecting. The confusion results in a negative malloc. Bad things follow. This re-writes the text console handling to be more robust. The key idea is that it should only look for a (qemu), once it has seen the original command echo'd back. This ensures it'll skip the bogus stray (qemu) with broken QEMUs. * src/qemu/qemu_monitor.c: Add some (disabled) debug code * src/qemu/qemu_monitor_text.c: Re-write way command replies are detected --- src/qemu/qemu_monitor.c | 28 ++++++++++ src/qemu/qemu_monitor_text.c | 102 +++++++++++++++++++++++------------ 2 files changed, 95 insertions(+), 35 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index abb763cfa2..103cf28fee 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -39,6 +39,8 @@ #define VIR_FROM_THIS VIR_FROM_QEMU +#define QEMU_DEBUG_RAW_IO 0 + struct _qemuMonitor { virMutex lock; virCond notify; @@ -163,6 +165,24 @@ char *qemuMonitorEscapeShell(const char *in) } +#if QEMU_DEBUG_RAW_IO +#include +static char * qemuMonitorEscapeNonPrintable(const char *text) +{ + int i; + virBuffer buf = VIR_BUFFER_INITIALIZER; + for (i = 0 ; text[i] != '\0' ; i++) { + if (c_isprint(text[i]) || + text[i] == '\n' || + (text[i] == '\r' && text[i+1] == '\n')) + virBufferVSprintf(&buf,"%c", text[i]); + else + virBufferVSprintf(&buf, "0x%02x", text[i]); + } + return virBufferContentAndReset(&buf); +} +#endif + void qemuMonitorLock(qemuMonitorPtr mon) { virMutexLock(&mon->lock); @@ -282,7 +302,15 @@ qemuMonitorIOProcess(qemuMonitorPtr mon) if (mon->msg && mon->msg->txOffset == mon->msg->txLength) msg = mon->msg; +#if QEMU_DEBUG_RAW_IO + char *str1 = qemuMonitorEscapeNonPrintable(msg ? msg->txBuffer : ""); + char *str2 = qemuMonitorEscapeNonPrintable(mon->buffer); + VIR_ERROR("Process %d %p %p [[[[%s]]][[[%s]]]", (int)mon->bufferOffset, mon->msg, msg, str1, str2); + VIR_FREE(str1); + VIR_FREE(str2); +#else VIR_DEBUG("Process %d", (int)mon->bufferOffset); +#endif if (mon->json) len = qemuMonitorJSONIOProcess(mon, mon->buffer, mon->bufferOffset, diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 0d5ad799d2..72cb2bb5a8 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -94,46 +94,78 @@ int qemuMonitorTextIOProcess(qemuMonitorPtr mon ATTRIBUTE_UNUSED, /* Look for a non-zero reply followed by prompt */ if (msg && !msg->finished) { - const char *end; + char *start = NULL; + char *end = NULL; + char *skip; - /* We might get a prompt for a password */ - end = strstr(data + used, PASSWORD_PROMPT); - if (end) { - VIR_DEBUG("Woooo passwowrd [%s]", data + used); - if (msg->passwordHandler) { - size_t consumed; - /* Try and handle the prompt */ - if (msg->passwordHandler(mon, msg, - data + used, - len - used, - msg->passwordOpaque) < 0) - return -1; - - /* Skip over prompt now */ - consumed = (end + strlen(PASSWORD_PROMPT)) - - (data + used); - used += consumed; - } else { - errno = EACCES; - return -1; + /* If we're here, we've already sent the command. We now + * strip the trailing '\r' because it makes the matching + * code that follows a little easier ie we can just strstr() + * for the original command + */ + if (msg->txLength > 0) { + char *tmp; + if ((tmp = strchr(msg->txBuffer, '\r'))) { + *tmp = '\0'; } } - /* We use the arrival of BASIC_PROMPT to detect when we've got a - * complete reply available from a command */ - end = strstr(data + used, BASIC_PROMPT); - if (end) { - /* QEMU echos the command back to us, full of control - * character junk that we don't want. Fortunately this - * is all terminated by LINE_ENDING, so we can easily - * skip over the control character junk */ - const char *start = strstr(data + used, LINE_ENDING); - if (!start) - start = data + used; - else - start += strlen(LINE_ENDING); - int want = end - start; + /* QEMU echos the command back to us, full of control + * character junk that we don't want. We have to skip + * over this junk by looking for the first complete + * repetition of our command. Then we can look for + * the prompt that is supposed to follow + * + * NB, we can't optimize by immediately looking for + * LINE_ENDING, because QEMU 0.10 has bad problems + * when initially connecting where it might write a + * prompt in the wrong place. So we must not look + * for LINE_ENDING, or BASIC_PROMPT until we've + * seen our original command echod. + */ + skip = strstr(data + used, msg->txBuffer); + /* After the junk we should have a line ending... */ + if (skip) { + start = strstr(skip + strlen(msg->txBuffer), LINE_ENDING); + } + + /* ... then our command reply data, following by a (qemu) prompt */ + if (start) { + char *passwd; + start += strlen(LINE_ENDING); + + /* We might get a prompt for a password before the (qemu) prompt */ + passwd = strstr(start, PASSWORD_PROMPT); + if (passwd) { + VIR_DEBUG("Seen a passwowrd prompt [%s]", data + used); + if (msg->passwordHandler) { + int i; + /* Try and handle the prompt */ + if (msg->passwordHandler(mon, msg, + start, + passwd - start + strlen(PASSWORD_PROMPT), + msg->passwordOpaque) < 0) + return -1; + + /* Blank out the password prompt so we don't re-trigger + * if we have to go back to sleep for more I/O */ + for (i = 0 ; i < strlen(PASSWORD_PROMPT) ; i++) + start[i] = ' '; + + /* Handled, so skip forward over password prompt */ + start = passwd; + } else { + errno = EACCES; + return -1; + } + } + + end = strstr(start, BASIC_PROMPT); + } + + if (start && end) { + int want = end - start; /* Annoyingly some commands may not have any reply data * at all upon success, but since we've detected the * BASIC_PROMPT we can reasonably reliably cope */