"Daniel P. Berrange" <berrange@xxxxxxxxxx> 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;
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list