"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