Re: [libvirt] [PATCH] eliminate strerror from qemu_driver.c: use virReportSystemError instead

On Thu, Jan 29, 2009 at 09:54:59PM +0100, Jim Meyering wrote: > diff --git a/src/qemu_driver.c b/src/qemu_driver.c You can actually just kill off the SetNonBlock method, since we added one to util.h. We should probably do same for SetCloseExec since I believe we have several copies of that with the sme problem too.

> @@ -854,8 +842,7 @@ static int qemudWaitForMonitor(virConnectPtr conn, > qemudFindCharDevicePTYs, > "console", 3000); > if (close(logfd) < 0) > - qemudLog(QEMUD_WARN, _("Unable to close logfile: %s\n"), > - strerror(errno)); > + virReportSystemError(NULL, errno, "%s", _("Unable to close logfile")); > return ret; > } If you report an error in this scenario then you must also ensure a failure code is returned. This particular scenario is non-fatal

though, so I don't think we should be raising an error here at all. I'd be inclined to just keep qemudLog, but with the raw errno value as an int.

> @@ -1134,30 +1121,30 @@ static int qemudStartVMDaemon(virConnectPtr conn, > tmp = progenv; > while (*tmp) { > if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0) > - qemudLog(QEMUD_WARN, _("Unable to write envv to logfile %d: %s\n"), > - errno, strerror(errno)); > + virReportSystemError(NULL, errno, > + "%s", _("Unable to write envv to logfile")); > if (safewrite(vm->logfile, " ", 1) < 0) > - qemudLog(QEMUD_WARN, _("Unable to write envv to logfile %d: %s\n"), > - errno, strerror(errno)); > + virReportSystemError(NULL, errno, > + "%s", _("Unable to write envv to logfile")); > tmp++; > } > tmp = argv; > while (*tmp) { > if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0) > - qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s\n"), > - errno, strerror(errno)); > + virReportSystemError(NULL, errno, > + "%s", _("Unable to write argv to logfile")); > if (safewrite(vm->logfile, " ", 1) < 0) > - qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s\n"), > - errno, strerror(errno)); > + virReportSystemError(NULL, errno, > + "%s", _("Unable to write argv to logfile")); > tmp++; > } > if (safewrite(vm->logfile, "\n", 1) < 0) > - qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s\n"), > - errno, strerror(errno)); > + virReportSystemError(NULL, errno, > + "%s", _("Unable to write argv to logfile")); > > if ((pos = lseek(vm->logfile, 0, SEEK_END)) < 0) > - qemudLog(QEMUD_WARN, _("Unable to seek to end of logfile %d: %s\n"), > - errno, strerror(errno)); > + virReportSystemError(NULL, errno, > + "%s", _("Unable to seek to end of logfile")); Likewise with all these - we're just printing ARGV to a logfile - this should not result in errors propagated to the caller - unless we intend to tear down the VM we just started, but I think that's overkill.

Hmm, these Log -> Error conversions all are same non-fatal scneario I mention earlier. > @@ -1492,7 +1480,7 @@ static int kvmGetMaxVCPUs(void) { > > fd = open(KVM_DEVICE, O_RDONLY); > if (fd < 0) { > - qemudLog(QEMUD_WARN, _("Unable to open %s: %s\n"), KVM_DEVICE, strerror(errno)); > + virReportSystemError(NULL, errno, _("Unable to open %s"), KVM_DEVICE); > return maxvcpus; > } > This conversion looks good - we SHOULD be raising a real error here. The original code was wrong to return 'maxvcpus' here, it should be -1 upon error.

From d02669442e0cd2354e6b429dbedcf665d15cfa87 Mon Sep 17 00:00:00 2001