[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 1/9] eliminate strerror qemu_driver.c: use virReportSystemError instead



"Daniel P. Berrange" <berrange 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;


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]