[libvirt] [PATCH] qemu: Don't parse log output when starting up a domain
Despite our great effort we still parsed qemu log output. We wouldn't notice unless qemu changed the format of the logs slightly. Anyway, now we should gather all interesting knobs like pty paths from monitor. Moreover, since for historical reasons the first console can be just an alias to the first serial port, we need to check this and copy the pty path if that's the case to the first console. --- src/qemu/qemu_capabilities.c | 7 +++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_process.c | 20 ++++++++++++++++++-- 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 17e6679..f49a31c 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1695,6 +1695,7 @@ qemuCapsPtr qemuCapsNewCopy(qemuCapsPtr caps) virBitmapCopy(ret->flags, caps->flags); + ret->usedQMP = caps->usedQMP; ret->version = caps->version; ret->kvmVersion = caps->kvmVersion; ret->arch = caps->arch; @@ -2633,3 +2634,9 @@ qemuCapsCacheFree(qemuCapsCachePtr cache) virMutexDestroy(&cache->lock); VIR_FREE(cache); } + +bool +qemuCapsUsedQMP(qemuCapsPtr caps) +{ + return caps->usedQMP; +} diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3457852..1417d7f 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -241,4 +241,5 @@ int qemuCapsParseDeviceStr(qemuCapsPtr caps, const char *str); VIR_ENUM_DECL(qemuCaps); +bool qemuCapsUsedQMP(qemuCapsPtr caps); #endif /* __QEMU_CAPABILITIES_H__*/ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index eac6553..ff88192 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1532,6 +1532,7 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, virHashTablePtr paths) { bool chardevfmt = qemuCapsGet(caps, QEMU_CAPS_CHARDEV); + int i = 0; if (qemuProcessLookupPTYs(vm->def->serials, vm->def->nserials, paths, chardevfmt) < 0) @@ -1544,8 +1545,23 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, if (qemuProcessLookupPTYs(vm->def->channels, vm->def->nchannels, paths, chardevfmt) < 0) return -1; + /* For historical reasons, console[0] can be just an alias + * for serial[0]; That's why we need to update it as well */ + if (vm->def->nconsoles) { + virDomainChrDefPtr chr = vm->def->consoles[0]; + + if (vm->def->nserials && + chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && + chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_SERIAL) { + /* yes, the first console is just an alias for serials[0] */ + i = 1; + if (virDomainChrSourceDefCopy(&chr->source, + &((vm->def->serials[0])->source)) < 0) + return -1; + } + } - if (qemuProcessLookupPTYs(vm->def->consoles, vm->def->nconsoles, + if (qemuProcessLookupPTYs(vm->def->consoles + i, vm->def->nconsoles - i, paths, chardevfmt) < 0) return -1; @@ -1650,7 +1666,7 @@ qemuProcessWaitForMonitor(virQEMUDriverPtr driver, virHashTablePtr paths = NULL; qemuDomainObjPrivatePtr priv; - if (pos != -1) { + if (!qemuCapsUsedQMP(caps) && pos != -1) { if ((logfd = qemuDomainOpenLog(driver, vm, pos)) < 0) return -1; -- 1.8.0.2
On 01/02/2013 08:23 AM, Michal Privoznik wrote:
Despite our great effort we still parsed qemu log output. We wouldn't notice unless qemu changed the format of the logs slightly.
Maybe mention that the upcoming qemu 1.4 does just that.
Anyway, now we should gather all interesting knobs like pty paths from monitor. Moreover, since for historical reasons the first console can be just an alias to the first serial port, we need to check this and copy the pty path if that's the case to the first console. --- src/qemu/qemu_capabilities.c | 7 +++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_process.c | 20 ++++++++++++++++++-- 3 files changed, 26 insertions(+), 2 deletions(-)
ACK, with a grammar nit.
@@ -1544,8 +1545,23 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, if (qemuProcessLookupPTYs(vm->def->channels, vm->def->nchannels, paths, chardevfmt) < 0) return -1; + /* For historical reasons, console[0] can be just an alias + * for serial[0]; That's why we need to update it as well */
Either you are starting a new sentence and need a full stop ("serial[0]. That's"), or you don't need a capital after semicolon ("serial[0]; that's"). Also, s/well/well./ wouldn't hurt. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 02.01.2013 18:45, Eric Blake wrote:
On 01/02/2013 08:23 AM, Michal Privoznik wrote:
Despite our great effort we still parsed qemu log output. We wouldn't notice unless qemu changed the format of the logs slightly.
Maybe mention that the upcoming qemu 1.4 does just that.
Anyway, now we should gather all interesting knobs like pty paths from monitor. Moreover, since for historical reasons the first console can be just an alias to the first serial port, we need to check this and copy the pty path if that's the case to the first console. --- src/qemu/qemu_capabilities.c | 7 +++++++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_process.c | 20 ++++++++++++++++++-- 3 files changed, 26 insertions(+), 2 deletions(-)
ACK, with a grammar nit.
@@ -1544,8 +1545,23 @@ qemuProcessFindCharDevicePTYsMonitor(virDomainObjPtr vm, if (qemuProcessLookupPTYs(vm->def->channels, vm->def->nchannels, paths, chardevfmt) < 0) return -1; + /* For historical reasons, console[0] can be just an alias + * for serial[0]; That's why we need to update it as well */
Either you are starting a new sentence and need a full stop ("serial[0]. That's"), or you don't need a capital after semicolon ("serial[0]; that's"). Also, s/well/well./ wouldn't hurt.
Fixed and pushed. Thanks. Michal
Commit b3f2b4ca5cfe98b08ffdb96f0455e3e333e5ace6 left buf unallocated in the case of QMP capability probing being used, leading to a segfault in strlen in the cleanup path. This patch opens the log and allocates the buffer if QMP probing was used, so we can display the helpful error message. --- src/qemu/qemu_process.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 358757b..2d63cf2 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1709,6 +1709,15 @@ cleanup: 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 */ + if (qemuCapsUsedQMP(caps)) { + if ((logfd = qemuDomainOpenLog(driver, vm, pos)) < 0) + return -1; + + if (VIR_ALLOC_N(buf, buf_size) < 0) { + virReportOOMError(); + goto closelog; + } + } qemuProcessReadLogFD(logfd, buf, buf_size, strlen(buf)); virReportError(VIR_ERR_INTERNAL_ERROR, _("process exited while connecting to monitor: %s"), -- 1.7.8.6
On 01/03/2013 11:15 AM, Ján Tomko wrote:
Commit b3f2b4ca5cfe98b08ffdb96f0455e3e333e5ace6 left buf unallocated in the case of QMP capability probing being used, leading to a segfault in strlen in the cleanup path.
This patch opens the log and allocates the buffer if QMP probing was used, so we can display the helpful error message. --- src/qemu/qemu_process.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 01/03/13 21:13, Eric Blake wrote:
On 01/03/2013 11:15 AM, Ján Tomko wrote:
Commit b3f2b4ca5cfe98b08ffdb96f0455e3e333e5ace6 left buf unallocated in the case of QMP capability probing being used, leading to a segfault in strlen in the cleanup path.
This patch opens the log and allocates the buffer if QMP probing was used, so we can display the helpful error message. --- src/qemu/qemu_process.c | 9 +++++++++ 1 files changed, 9 insertions(+), 0 deletions(-)
ACK.
Thanks, pushed.
participants (3)
-
Eric Blake -
Ján Tomko -
Michal Privoznik