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

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

 



"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
...
>> -    qemudLog(QEMUD_ERR,
>> -             "%s", _("Failed to set non-blocking file descriptor flag\n"));
>> -    return -1;
>> +    return ((flags = fcntl(fd, F_GETFL)) < 0
>> +             || fcntl(fd, F_SETFL, flags | O_NONBLOCK) < 0
>> +            ? -1
>> +            : 0);
>>  }
>
> These two chunks can be left out, since those functions are completely
> removed by the next patch.
>
>
>> @@ -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.

Worst still, there are two symbols used here,

  QEMUD_ERR
  QEMUD_ERROR

neither of which is defined.
That's technically fine, since they're both
preprocessed away, but makes searching for
occurrences (like I was just doing) prone to error
if you happen to use a word-restricted search or search
only for the longer string.

I'll normalize to QEMUD_ERROR while I'm here.

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]