On 08/05/2011 04:17 AM, Eric Blake wrote:
On 08/04/2011 02:07 PM, Eric Blake wrote:
We definitely have a bug here, but this is not the right fix. The bug is that the cleanup: label is trying to read from logfd if the vm crashed, without having opened logfd in the qemuProcessAttach case.
I think the more appropriate patch is this:
diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c index 8508ff6..1eea45f 100644 --- i/src/qemu/qemu_process.c +++ w/src/qemu/qemu_process.c @@ -1214,7 +1214,7 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver, cleanup: virHashFree(paths);
- if (kill(vm->pid, 0) == -1 && errno == ESRCH) { + if (pos != -1 && kill(vm->pid, 0) == -1 && errno == ESRCH) { /* VM is dead, any other error raised in the interim is probably * not as important as the qemu cmdline output */ qemuProcessReadLogFD(logfd, buf, buf_size, strlen(buf));
Also squash this in - no need to free buf twice.
diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c index 1eea45f..a37f709 100644 --- i/src/qemu/qemu_process.c +++ w/src/qemu/qemu_process.c @@ -1221,16 +1221,14 @@ cleanup: qemuReportError(VIR_ERR_INTERNAL_ERROR, _("process exited while connecting to monitor: %s"), buf); ret = -1; }
closelog: - VIR_FREE(buf); - if (VIR_CLOSE(logfd) < 0) { char ebuf[1024]; VIR_WARN("Unable to close logfile: %s", virStrerror(errno, ebuf, sizeof ebuf)); }
VIR_FREE(buf);
Right, this is a double free, Eric, Hasn't Coverity found this issue? Thanks, Alex