[libvirt] [PATCH] Sanitize qemu monitor reads

[Cole Robinson <crobinso@xxxxxxxxxx>]

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

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 9d8f75a..87c9be7 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -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);
+            }
+
             break;
         }
     pollagain:
@@ -3103,7 +3127,7 @@ static int qemudDomainChangeEjectableMedia(virDomainPtr dom,
 
     if (qemudMonitorCommand(driver, vm, cmd, &reply) < 0) {
         qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                         "%s", _("cannot change cdrom media"));
+                         "%s", _("could not change cdrom media"));
         VIR_FREE(cmd);
         return -1;
     }
@@ -3114,7 +3138,7 @@ static int qemudDomainChangeEjectableMedia(virDomainPtr dom,
     DEBUG ("ejectable media change reply: %s", reply);
     if (strstr(reply, "\ndevice ")) {
         qemudReportError (dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
-                          "%s", _("changing cdrom media failed"));
+                          _("changing cdrom media failed: %s"), reply);
         VIR_FREE(reply);
         VIR_FREE(cmd);
         return -1;

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list