Re: [libvirt] [PATCH 1/9] eliminate strerror qemu_driver.c: use virReportSystemError instead
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
...
These two chunks can be left out, since those functions are
completely
removed by the next patch.
Sure. No net change.
> @@ -877,8 +865,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"));
This is not fatal to starting the VM, so should raise an
error here. Could argue we shoud raise the log level to
QEMUD_ERROR though.
> @@ -1200,30 +1187,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"));
...
Likewise all of those are non-fatal, so shouldn't be raising an
error back to the caller, since the overall operation is still
reporting success code.
Ok. I'll leave all "qemudLog(QEMUD_WARN" uses (and the one,
just-below use of QEMUD_ERROR-that-should-be-WARN) as _WARN
and just replace strerror/virStrerror, as done in subsequent patches.
This means reordering this patch to follow the
one that publicizes virStrerror. No problem, of course.
> @@ -1300,8 +1288,9 @@ static void
qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
>
> if (virKillProcess(vm->pid, 0) == 0 &&
> virKillProcess(vm->pid, SIGTERM) < 0)
> - qemudLog(QEMUD_ERROR, _("Failed to send SIGTERM to %s (%d):
%s\n"),
> - vm->def->name, vm->pid, strerror(errno));
> + virReportSystemError(conn, errno,
> + _("Failed to send SIGTERM to %s (%d)"),
> + vm->def->name, vm->pid);
...
> @@ -1593,7 +1581,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 needs to report a real error code back to the user, since if
we are using KVM vms, then /dev/kvm should always exist.
Ok. I'll make it fail like this, then:
- return maxvcpus;
+ return -1;