On Mon, Feb 14, 2011 at 04:59:39PM -0700, Eric Blake wrote:

The processWatchdogEvent fix is real, although it can only trigger on OOM, since bad things happen if doCoreDump is called with a NULL pathname argument. The other fixes silence clang, but aren't a real bug because virReportErrorHelper tolerates a NULL format string even though *printf does not.

* src/qemu/qemu_driver.c (processWatchdogEvent): Exit on OOM. (qemuDomainIsActive, qemuDomainIsPersistent, qemuDomainIsUpdated): Provide valid message. ---

Another valid clang finding. Maybe we whould mark virReportErrorHelper as ATTRIBUTE_NONNULL for the format string argument, and require all clients to pass in a valid error string (after all, that's what the printf attritube already assumes, and passing NULL just means that we aren't giving back any useful error information to the user); but that can be a separate cleanup.

src/qemu/qemu_driver.c | 23 +++++++++++++++++------ 1 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c581cfe..82a2210 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -3489,7 +3489,10 @@ static int qemuDomainIsActive(virDomainPtr dom) obj = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); if (!obj) { - qemuReportError(VIR_ERR_NO_DOMAIN, NULL); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } ret = virDomainObjIsActive(obj); @@ -3510,7 +3513,10 @@ static int qemuDomainIsPersistent(virDomainPtr dom) obj = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); if (!obj) { - qemuReportError(VIR_ERR_NO_DOMAIN, NULL); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } ret = obj->persistent; @@ -3531,7 +3537,10 @@ static int qemuDomainIsUpdated(virDomainPtr dom) obj = virDomainFindByUUID(&driver->domains, dom->uuid); qemuDriverUnlock(driver); if (!obj) { - qemuReportError(VIR_ERR_NO_DOMAIN, NULL); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virUUIDFormat(dom->uuid, uuidstr); + qemuReportError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); goto cleanup; } ret = obj->updated; @@ -4981,12 +4990,14 @@ static void processWatchdogEvent(void *data, void *opaque) case VIR_DOMAIN_WATCHDOG_ACTION_DUMP: { char *dumpfile; - int i;

- i = virAsprintf(&dumpfile, "%s/%s-%u", + if (virAsprintf(&dumpfile, "%s/%s-%u", driver->autoDumpPath, wdEvent->vm->def->name, - (unsigned int)time(NULL)); + (unsigned int)time(NULL)) < 0) { + virReportOOMError(); + break; + }

qemuDriverLock(driver); virDomainObjLock(wdEvent->vm);

ACK, fine pushing now, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/