Re: [libvirt] [PATCH] Sanitize qemu monitor reads

[Cole Robinson <crobinso redhat com>]

Richard W.M. Jones wrote:
> 
> It looks to me like tmpbuf is leaked on the two error paths.
> 
> garbage.collection++
> 
> Rich.
> 

ah foo, sorry about that. Fixed version attached.

Thanks,
Cole

diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 9d8f75a..b7c8e70 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,30 @@ 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))) {
+                if ((nlptr = strchr(commptr, '\n'))) {
+                    if (VIR_ALLOC_N(tmpbuf, strlen(cmd)+strlen(nlptr)+1) < 0)
+                        goto error;
+                    strncpy(tmpbuf, cmd, strlen(cmd));
+                    strcat(tmpbuf, nlptr);
+                } else {
+                    if ((tmpbuf = strdup(commptr)) == NULL)
+                        goto error;
+                }
+                VIR_FREE(buf);
+                buf = tmpbuf;
+            }
+
             break;
         }
     pollagain:
@@ -3103,7 +3126,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 +3137,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;