3:56 p.m.
Reading from the qemu monitor pulls in a whole bunch of
useless control characters. For example, sending the
command 'somecomm' to the monitor returns:
somecomm
unknown command: 'somecomm'
(qemu)
Which is 36 characters, however we end up reading over 200.
The amount we read actually grows quadratically as a function
of the command length. This prevents us from reporting monitor
output to the user (incase some command fails) and seriously
diminishes the value of the domain logfiles.
The attached patch tries to sanitize this a bit. After reading
all the output, we search for the first occurrence of the full
command string. If found, we search from there for the first
newline (which marks the beginning of the monitor response).
If these are found, we only return the command, and everything
after the newline. If only the command is found, we just drop
everything before it.
I've tested this on f9 and f8, there didn't seem to be any
problems. I think this should be pretty future proof as
well, so we hopefully won't be throwing out anything
valuable.
Thanks,
Cole
6:18 a.m.
On Tue, Sep 23, 2008 at 04:56:08PM -0400, Cole Robinson wrote:
@@ -1670,7 +1670,7 @@ qemudMonitorCommand (const struct qemud_driver
*driver ATTRIBUTE_UNUSED,
const char *cmd,
char **reply) {
int size = 0;
- char *buf = NULL;
+ char *buf = NULL, *tmpbuf = NULL, *nlptr = NULL, *commptr = NULL;
size_t cmdlen = strlen(cmd);
if (safewrite(vm->monitor, cmd, cmdlen) != cmdlen)
@@ -1708,7 +1708,31 @@ qemudMonitorCommand (const struct qemud_driver *driver
ATTRIBUTE_UNUSED,
/* Look for QEMU prompt to indicate completion */
if (buf && ((tmp = strstr(buf, "\n(qemu) ")) != NULL)) {
- tmp[0] = '\0';
+ /* Preserve the newline */
+ tmp[1] = '\0';
+
+ /* The monitor doesn't dump clean output after we have written to
+ * it. Every character we write dumps a bunch of useless stuff,
+ * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand"
+ * Try to throw away everything before the first full command
+ * occurence, and inbetween the command and the newline starting
+ * the response
+ */
+ if ((commptr = strstr(buf, cmd))) {
+ tmpbuf = buf;
+ buf = NULL;
+ if ((nlptr = strchr(commptr, '\n'))) {
+ if (VIR_ALLOC_N(buf, strlen(cmd) + strlen(nlptr) + 1) < 0)
+ goto error;
+ strncpy(buf, cmd, strlen(cmd));
+ strcat(buf, nlptr);
+ } else {
+ if ((buf = strdup(commptr)) == NULL)
+ goto error;
+ }
+ VIR_FREE(tmpbuf);
+ }
It looks to me like tmpbuf is leaked on the two error paths.
garbage.collection++
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
5904
days inactive
5905
days old
2 comments
2 participants
participants (2)
-
Cole Robinson -
Richard W.M. Jones