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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list