10:37 a.m.
We know that strerror is not thread-safe, so I'm
eliminating all uses from code that might be multi-threaded.
I started the job with this patch:
http://thread.gmane.org/gmane.comp.emulators.libvirt/11532
Here are the summaries of the upcoming change sets:
eliminate strerror qemu_driver.c: use virReportSystemError instead
This is the patch mentioned above.
remove duplicate *SetCloseExec and *SetNonBlock functions
Changes prompted by Dan's feedback (see thread).
report OOMError
tiny
iptables.c: Use virStrerror, not strerror.
qemud.c: use virStrerror, not strerror
don't include raw errno in diagnostics
remove remainder of offending strerror uses
note: the qemud/remote.c changes that move virDomainFree calls
do that merely to make that code identical to a nearby, nearly-
identical function.
syntax-check: enable prohibit_nonreentrant
makes "make syntax-check" check for all those non-reentrant
functions, but excluding virterror.c, virsh.c and console.c
syntax-check: avoid spurious false-positive
11:08 a.m.
New subject: [libvirt] [PATCH 1/9] eliminate strerror qemu_driver.c: use virReportSystemError instead
From: Jim Meyering <meyering(a)redhat.com>
* src/qemu_driver.c (qemudSetCloseExec): Don't use qemudLog here.
Now, every caller diagnoses the failure.
Simplify, now that there's no logging.
* src/qemu_driver.c (qemudSetNonBlock): Rewrite not to use qemudLog.
---
src/qemu_driver.c | 126 ++++++++++++++++++++++++-----------------------------
1 files changed, 57 insertions(+), 69 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 09f69bf..596d940 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -93,31 +93,19 @@ static void qemuDriverUnlock(struct qemud_driver *driver)
static int qemudSetCloseExec(int fd) {
int flags;
- if ((flags = fcntl(fd, F_GETFD)) < 0)
- goto error;
- flags |= FD_CLOEXEC;
- if ((fcntl(fd, F_SETFD, flags)) < 0)
- goto error;
- return 0;
- error:
- qemudLog(QEMUD_ERR,
- "%s", _("Failed to set close-on-exec file descriptor
flag\n"));
- return -1;
+ return ((flags = fcntl(fd, F_GETFD)) < 0
+ || fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0
+ ? -1
+ : 0);
}
static int qemudSetNonBlock(int fd) {
int flags;
- if ((flags = fcntl(fd, F_GETFL)) < 0)
- goto error;
- flags |= O_NONBLOCK;
- if ((fcntl(fd, F_SETFL, flags)) < 0)
- goto error;
- return 0;
- error:
- 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);
}
@@ -207,22 +195,21 @@ qemudLogReadFD(virConnectPtr conn, const char* logDir, const char*
name, off_t p
if ((fd = open(logfile, logmode)) < 0) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("failed to create logfile %s: %s"),
- logfile, strerror(errno));
+ virReportSystemError(conn, errno,
+ _("failed to create logfile %s"),
+ logfile);
return -1;
}
if (qemudSetCloseExec(fd) < 0) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("Unable to set VM logfile close-on-exec flag: %s"),
- strerror(errno));
+ virReportSystemError(conn, errno, "%s",
+ _("Unable to set VM logfile close-on-exec
flag"));
close(fd);
return -1;
}
if (lseek(fd, pos, SEEK_SET) < 0) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("Unable to seek to %lld in %s: %s"),
- (long long) pos, logfile, strerror(errno));
+ virReportSystemError(conn, errno,
+ _("Unable to seek to %lld in %s"),
+ (long long) pos, logfile);
close(fd);
}
return fd;
@@ -451,8 +438,9 @@ qemudStartup(void) {
}
if (virFileMakePath(qemu_driver->stateDir) < 0) {
- qemudLog(QEMUD_ERR, _("Failed to create state dir '%s':
%s\n"),
- qemu_driver->stateDir, strerror(errno));
+ virReportSystemError(NULL, errno,
+ _("Failed to create state dir '%s'"),
+ qemu_driver->stateDir);
goto error;
}
@@ -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"));
if (ret == 1) /* Success */
return 0;
@@ -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"));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to write argv to
logfile"));
if (safewrite(vm->logfile, " ", 1) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to write argv to
logfile"));
tmp++;
}
if (safewrite(vm->logfile, "\n", 1) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s\n"),
- errno, strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to write argv to
logfile"));
if ((pos = lseek(vm->logfile, 0, SEEK_END)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to seek to end of logfile %d: %s\n"),
- errno, strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to seek to end of
logfile"));
for (i = 0 ; i < ntapfds ; i++)
FD_SET(tapfds[i], &keepfd);
@@ -1292,7 +1279,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
- struct qemud_driver *driver, virDomainObjPtr vm) {
+ struct qemud_driver *driver,
+ virDomainObjPtr vm) {
if (!virDomainIsActive(vm))
return;
@@ -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);
if (vm->monitor_watch != -1) {
virEventRemoveHandle(vm->monitor_watch);
@@ -1309,8 +1298,7 @@ static void qemudShutdownVMDaemon(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
if (close(vm->logfile) < 0)
- qemudLog(QEMUD_WARN, _("Unable to close logfile %d: %s\n"),
- errno, strerror(errno));
+ virReportSystemError(conn, errno, "%s", _("Unable to close
logfile"));
if (vm->monitor != -1)
close(vm->monitor);
vm->logfile = -1;
@@ -1477,8 +1465,8 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm,
/* Log, but ignore failures to write logfile for VM */
if (safewrite(vm->logfile, buf, strlen(buf)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s\n"),
- strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to log VM console
data"));
*reply = buf;
return 0;
@@ -1487,8 +1475,8 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm,
if (buf) {
/* Log, but ignore failures to write logfile for VM */
if (safewrite(vm->logfile, buf, strlen(buf)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s\n"),
- strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to log VM console
data"));
VIR_FREE(buf);
}
return -1;
@@ -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;
}
@@ -1836,8 +1824,8 @@ qemudGetHostname (virConnectPtr conn)
result = virGetHostname();
if (result == NULL) {
- qemudReportError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (conn, errno,
+ "%s", _("failed to determine host
name"));
return NULL;
}
/* Caller frees this string. */
@@ -3922,8 +3910,8 @@ qemudDomainBlockPeek (virDomainPtr dom,
/* The path is correct, now try to open it and get its size. */
fd = open (path, O_RDONLY);
if (fd == -1) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dom->conn, errno,
+ _("%s: failed to open"), path);
goto cleanup;
}
@@ -3933,8 +3921,8 @@ qemudDomainBlockPeek (virDomainPtr dom,
*/
if (lseek (fd, offset, SEEK_SET) == (off_t) -1 ||
saferead (fd, buffer, size) == (ssize_t) -1) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dom->conn, errno,
+ _("%s: failed to seek or read"), path);
goto cleanup;
}
@@ -3988,8 +3976,8 @@ qemudDomainMemoryPeek (virDomainPtr dom,
/* Create a temporary filename. */
if ((fd = mkstemp (tmp)) == -1) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dom->conn, errno,
+ _("mkstemp(\"%s\") failed"), tmp);
goto cleanup;
}
@@ -4005,8 +3993,9 @@ qemudDomainMemoryPeek (virDomainPtr dom,
/* Read the memory file into buffer. */
if (saferead (fd, buffer, size) == (ssize_t) -1) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dom->conn, errno,
+ _("failed to read temporary file "
+ "created with template %s"), tmp);
goto cleanup;
}
@@ -4165,8 +4154,8 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
/* Get hostname */
if (gethostname (hostname, HOST_NAME_MAX+1) == -1) {
- qemudReportError (dconn, NULL, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dconn, errno,
+ "%s", _("failed to determine host
name"));
goto cleanup;
}
@@ -4336,8 +4325,7 @@ qemudDomainMigratePerform (virDomainPtr dom,
/* Issue the migrate command. */
safe_uri = qemudEscapeMonitorArg (uri);
if (!safe_uri) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportOOMError (dom->conn);
goto cleanup;
}
snprintf (cmd, sizeof cmd, "migrate \"%s\"", safe_uri);
--
1.6.1.2.418.gd79e6
8:08 a.m.
New subject: [libvirt] [PATCH 1/9] eliminate strerror qemu_driver.c: use virReportSystemError instead
On Mon, Feb 02, 2009 at 06:08:14PM +0100, Jim Meyering wrote:
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 09f69bf..596d940 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -93,31 +93,19 @@ static void qemuDriverUnlock(struct qemud_driver *driver)
static int qemudSetCloseExec(int fd) {
int flags;
- if ((flags = fcntl(fd, F_GETFD)) < 0)
- goto error;
- flags |= FD_CLOEXEC;
- if ((fcntl(fd, F_SETFD, flags)) < 0)
- goto error;
- return 0;
- error:
- qemudLog(QEMUD_ERR,
- "%s", _("Failed to set close-on-exec file descriptor
flag\n"));
- return -1;
+ return ((flags = fcntl(fd, F_GETFD)) < 0
+ || fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0
+ ? -1
+ : 0);
}
static int qemudSetNonBlock(int fd) {
int flags;
- if ((flags = fcntl(fd, F_GETFL)) < 0)
- goto error;
- flags |= O_NONBLOCK;
- if ((fcntl(fd, F_SETFL, flags)) < 0)
- goto error;
- return 0;
- error:
- 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.
@@ -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"));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to write argv to
logfile"));
if (safewrite(vm->logfile, " ", 1) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to write argv to
logfile"));
tmp++;
}
if (safewrite(vm->logfile, "\n", 1) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s\n"),
- errno, strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to write argv to
logfile"));
if ((pos = lseek(vm->logfile, 0, SEEK_END)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to seek to end of logfile %d: %s\n"),
- errno, strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to seek to end of
logfile"));
for (i = 0 ; i < ntapfds ; i++)
FD_SET(tapfds[i], &keepfd);
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.
@@ -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);
if (vm->monitor_watch != -1) {
virEventRemoveHandle(vm->monitor_watch);
@@ -1309,8 +1298,7 @@ static void qemudShutdownVMDaemon(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
if (close(vm->logfile) < 0)
- qemudLog(QEMUD_WARN, _("Unable to close logfile %d: %s\n"),
- errno, strerror(errno));
+ virReportSystemError(conn, errno, "%s", _("Unable to close
logfile"));
if (vm->monitor != -1)
close(vm->monitor);
vm->logfile = -1;
These two are both non-fatal to the proces of shutting down
the VM daemon. In facton the 2nd case, I'm wondering why we
still have the vm->logfile handle open at all. We are no longer
responsible for logging stdout/err from the QEMU proess into
the logfile, since we dup'd its stdout/err directly onto the
logfile FD. So we shouldnt' have any reason to keep this open.
@@ -1477,8 +1465,8 @@ qemudMonitorCommandExtra(const virDomainObjPtr
vm,
/* Log, but ignore failures to write logfile for VM */
if (safewrite(vm->logfile, buf, strlen(buf)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s\n"),
- strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to log VM console
data"));
*reply = buf;
return 0;
@@ -1487,8 +1475,8 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm,
if (buf) {
/* Log, but ignore failures to write logfile for VM */
if (safewrite(vm->logfile, buf, strlen(buf)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s\n"),
- strerror(errno));
+ virReportSystemError(NULL, errno,
+ "%s", _("Unable to log VM console
data"));
VIR_FREE(buf);
}
return -1;
These two should stay as logging calls too, since they're not
fatal.
@@ -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.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:41 a.m.
New subject: [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;
2:13 p.m.
New subject: [libvirt] [PATCH 1/9] eliminate strerror qemu_driver.c: use virReportSystemError instead
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
...
> @@ -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.
FYI, whether we use QEMUD_ERROR or QEMUD_WARN doesn't really
matter, since that parameter is ignored by the qemudLog macro:
#define qemudLog(level, msg...) fprintf(stderr, msg)
3:37 p.m.
New subject: [libvirt] [PATCH 1/9] eliminate strerror qemu_driver.c: use virReportSystemError instead
On Tue, Feb 03, 2009 at 09:13:04PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
...
>> @@ -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.
FYI, whether we use QEMUD_ERROR or QEMUD_WARN doesn't really
matter, since that parameter is ignored by the qemudLog macro:
#define qemudLog(level, msg...) fprintf(stderr, msg)
Oh, I thought DV had already switched this macro over to using the
definitions from src/logging.h, which does pay attention to "level" arg.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
2:18 p.m.
New subject: [libvirt] [PATCH 1/9] eliminate strerror qemu_driver.c: use virReportSystemError instead
"Daniel P. Berrange" <berrange(a)redhat.com> 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.
11:08 a.m.
New subject: [libvirt] [PATCH 2/9] remove duplicate *SetCloseExec and *SetNonBlock functions
From: Jim Meyering <meyering(a)redhat.com>
* src/qemu_driver.c: Use virSetCloseExec and virSetNonBlock,
rather than qemuSet* functions. Suggested by Daniel P. Berrange.
* src/util.c (virSetCloseExec): Compile unconditionally.
* src/util.h (virSetCloseExec): Declare
libvirtd: link with libvirt_util, too
* qemud/Makefile.am (libvirtd_LDADD): Add ../src/libvirt_util.la,
for use of virSetCloseExec.
---
qemud/Makefile.am | 1 +
src/qemu_driver.c | 27 ++++-----------------------
src/util.c | 9 ++++-----
src/util.h | 1 +
4 files changed, 10 insertions(+), 28 deletions(-)
diff --git a/qemud/Makefile.am b/qemud/Makefile.am
index a0c161a..372b931 100644
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -107,6 +107,7 @@ libvirtd_LDADD = \
if ! WITH_DRIVER_MODULES
if WITH_QEMU
libvirtd_LDADD += ../src/libvirt_driver_qemu.la
+libvirtd_LDADD += ../src/libvirt_util.la
endif
if WITH_LXC
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 596d940..c40fda4 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -91,25 +91,6 @@ static void qemuDriverUnlock(struct qemud_driver *driver)
virMutexUnlock(&driver->lock);
}
-static int qemudSetCloseExec(int fd) {
- int flags;
- return ((flags = fcntl(fd, F_GETFD)) < 0
- || fcntl(fd, F_SETFD, flags | FD_CLOEXEC) < 0
- ? -1
- : 0);
-}
-
-
-static int qemudSetNonBlock(int fd) {
- int flags;
- return ((flags = fcntl(fd, F_GETFL)) < 0
- || fcntl(fd, F_SETFL, flags | O_NONBLOCK) < 0
- ? -1
- : 0);
-}
-
-
-
static void qemuDomainEventFlush(int timer, void *opaque);
static void qemuDomainEventQueue(struct qemud_driver *driver,
virDomainEventPtr event);
@@ -168,7 +149,7 @@ qemudLogFD(virConnectPtr conn, const char* logDir, const char* name)
logfile);
return -1;
}
- if (qemudSetCloseExec(fd) < 0) {
+ if (virSetCloseExec(fd) < 0) {
virReportSystemError(conn, errno, "%s",
_("Unable to set VM logfile close-on-exec
flag"));
close(fd);
@@ -200,7 +181,7 @@ qemudLogReadFD(virConnectPtr conn, const char* logDir, const char*
name, off_t p
logfile);
return -1;
}
- if (qemudSetCloseExec(fd) < 0) {
+ if (virSetCloseExec(fd) < 0) {
virReportSystemError(conn, errno, "%s",
_("Unable to set VM logfile close-on-exec
flag"));
close(fd);
@@ -710,12 +691,12 @@ static int qemudOpenMonitor(virConnectPtr conn,
_("Unable to open monitor path %s"), monitor);
return -1;
}
- if (qemudSetCloseExec(monfd) < 0) {
+ if (virSetCloseExec(monfd) < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Unable to set monitor close-on-exec
flag"));
goto error;
}
- if (qemudSetNonBlock(monfd) < 0) {
+ if (virSetNonBlock(monfd) < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Unable to put monitor into non-blocking
mode"));
goto error;
diff --git a/src/util.c b/src/util.c
index 7c7fe31..6c29872 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1,7 +1,7 @@
/*
* utils.c: common, generic utility functions
*
- * Copyright (C) 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
* Copyright (C) 2006, 2007 Binary Karma
* Copyright (C) 2006 Shuveb Hussain
@@ -182,10 +182,7 @@ int virSetNonBlock(int fd) {
return 0;
}
-
-#ifndef WIN32
-
-static int virSetCloseExec(int fd) {
+int virSetCloseExec(int fd) {
int flags;
if ((flags = fcntl(fd, F_GETFD)) < 0)
return -1;
@@ -195,6 +192,8 @@ static int virSetCloseExec(int fd) {
return 0;
}
+#ifndef WIN32
+
static int
__virExec(virConnectPtr conn,
const char *const*argv,
diff --git a/src/util.h b/src/util.h
index e731ba4..62df202 100644
--- a/src/util.h
+++ b/src/util.h
@@ -39,6 +39,7 @@ enum {
};
int virSetNonBlock(int fd);
+int virSetCloseExec(int fd);
int virExec(virConnectPtr conn,
const char *const*argv,
--
1.6.1.2.418.gd79e6
7:55 a.m.
New subject: [libvirt] [PATCH 2/9] remove duplicate *SetCloseExec and *SetNonBlock functions
On Mon, Feb 02, 2009 at 06:08:15PM +0100, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
* src/qemu_driver.c: Use virSetCloseExec and virSetNonBlock,
rather than qemuSet* functions. Suggested by Daniel P. Berrange.
* src/util.c (virSetCloseExec): Compile unconditionally.
* src/util.h (virSetCloseExec): Declare
diff --git a/qemud/Makefile.am b/qemud/Makefile.am
index a0c161a..372b931 100644
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -107,6 +107,7 @@ libvirtd_LDADD = \
if ! WITH_DRIVER_MODULES
if WITH_QEMU
libvirtd_LDADD += ../src/libvirt_driver_qemu.la
+libvirtd_LDADD += ../src/libvirt_util.la
endif
This is not required. All the functions in util.c are provided
in the libvirt.so, and exported with LIBVIRT_PRIVATE_X_X_X
tag.
This patch is missing the change to libvirt_private.sym to
actually add the virSetCloseExec function though.
@@ -182,10 +182,7 @@ int virSetNonBlock(int fd) {
return 0;
}
-
-#ifndef WIN32
-
-static int virSetCloseExec(int fd) {
+int virSetCloseExec(int fd) {
int flags;
if ((flags = fcntl(fd, F_GETFD)) < 0)
return -1;
@@ -195,6 +192,8 @@ static int virSetCloseExec(int fd) {
return 0;
}
+#ifndef WIN32
+
static int
__virExec(virConnectPtr conn,
const char *const*argv,
Why this change ? AFAIK, there is no close-on-exec flag in Win32,
since it doesn't have any concept of exec(). The QEMU driver isn't
built on Win32, so we shouldn't need to expose virSetCloseExec
there anyway.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
10:06 a.m.
New subject: [libvirt] [PATCH 2/9] remove duplicate *SetCloseExec and *SetNonBlock functions
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Mon, Feb 02, 2009 at 06:08:15PM +0100, Jim Meyering wrote:
> From: Jim Meyering <meyering(a)redhat.com>
>
> * src/qemu_driver.c: Use virSetCloseExec and virSetNonBlock,
> rather than qemuSet* functions. Suggested by Daniel P. Berrange.
> * src/util.c (virSetCloseExec): Compile unconditionally.
> * src/util.h (virSetCloseExec): Declare
>
> diff --git a/qemud/Makefile.am b/qemud/Makefile.am
> index a0c161a..372b931 100644
> --- a/qemud/Makefile.am
> +++ b/qemud/Makefile.am
> @@ -107,6 +107,7 @@ libvirtd_LDADD = \
> if ! WITH_DRIVER_MODULES
> if WITH_QEMU
> libvirtd_LDADD += ../src/libvirt_driver_qemu.la
> +libvirtd_LDADD += ../src/libvirt_util.la
> endif
This is not required. All the functions in util.c are provided
in the libvirt.so, and exported with LIBVIRT_PRIVATE_X_X_X
tag.
This patch is missing the change to libvirt_private.sym to
actually add the virSetCloseExec function though.
Ah. forgot about that.
Done.
> @@ -182,10 +182,7 @@ int virSetNonBlock(int fd) {
> return 0;
> }
>
> -
> -#ifndef WIN32
> -
> -static int virSetCloseExec(int fd) {
> +int virSetCloseExec(int fd) {
> int flags;
> if ((flags = fcntl(fd, F_GETFD)) < 0)
> return -1;
> @@ -195,6 +192,8 @@ static int virSetCloseExec(int fd) {
> return 0;
> }
>
> +#ifndef WIN32
> +
> static int
> __virExec(virConnectPtr conn,
> const char *const*argv,
Why this change ? AFAIK, there is no close-on-exec flag in Win32,
since it doesn't have any concept of exec(). The QEMU driver isn't
built on Win32, so we shouldn't need to expose virSetCloseExec
there anyway.
Looked like it'd be portable enough to compile even there,
plus the general preference to avoid ifdef'd code in libraries.
I've read up a little and see that there really is no point,
so have put it back.
11:08 a.m.
New subject: [libvirt] [PATCH 3/9] report OOMError
From: Jim Meyering <meyering(a)redhat.com>
---
src/qemu_driver.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index c40fda4..599af0b 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -462,8 +462,7 @@ qemudStartup(void) {
return 0;
out_of_memory:
- qemudLog (QEMUD_ERR,
- "%s", _("qemudStartup: out of memory\n"));
+ virReportOOMError(NULL);
error:
if (qemu_driver)
qemuDriverUnlock(qemu_driver);
--
1.6.1.2.418.gd79e6
7:56 a.m.
New subject: [libvirt] [PATCH 3/9] report OOMError
On Mon, Feb 02, 2009 at 06:08:16PM +0100, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
---
src/qemu_driver.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index c40fda4..599af0b 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -462,8 +462,7 @@ qemudStartup(void) {
return 0;
out_of_memory:
- qemudLog (QEMUD_ERR,
- "%s", _("qemudStartup: out of memory\n"));
+ virReportOOMError(NULL);
error:
if (qemu_driver)
qemuDriverUnlock(qemu_driver);
ACk
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:08 a.m.
New subject: [libvirt] [PATCH 4/9] iptables.c: Use virStrerror, not strerror.
From: Jim Meyering <meyering(a)redhat.com>
* src/iptables.c: Include "virterror_internal.h".
Use virStrerror, not strerror.
* src/virterror.c (virStrerror): Remove "static".
* src/virterror_internal.h (virStrerror): Declare it.
---
src/iptables.c | 25 ++++++++++++++++---------
src/virterror.c | 2 +-
src/virterror_internal.h | 3 ++-
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/iptables.c b/src/iptables.c
index ad7fddf..81c976b 100644
--- a/src/iptables.c
+++ b/src/iptables.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2007-2009 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -44,6 +44,7 @@
#include "iptables.h"
#include "util.h"
#include "memory.h"
+#include "virterror_internal.h"
#define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -98,9 +99,10 @@ notifyRulesUpdated(const char *table,
argv[2] = arg;
argv[3] = NULL;
+ char ebuf[1024];
if (virRun(NULL, argv, NULL) < 0)
- qemudLog(QEMUD_WARN, _("Failed to run '" LOKKIT_PATH
- " %s' : %s"), arg, strerror(errno));
+ qemudLog(QEMUD_WARN, _("Failed to run '" LOKKIT_PATH "
%s': %s"),
+ arg, virStrerror(errno, ebuf, sizeof ebuf));
}
static int
@@ -174,10 +176,11 @@ notifyRulesRemoved(const char *table,
return;
- write_error:
+ write_error:;
+ char ebuf[1024];
qemudLog(QEMUD_WARN, _("Failed to write to " SYSCONF_DIR
"/sysconfig/system-config-firewall : %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
if (f)
fclose(f);
VIR_FREE(content);
@@ -239,15 +242,16 @@ iptRulesSave(iptRules *rules)
#ifdef ENABLE_IPTABLES_LOKKIT
int err;
+ char ebuf[1024];
if ((err = virFileMakePath(rules->dir))) {
qemudLog(QEMUD_WARN, _("Failed to create directory %s : %s"),
- rules->dir, strerror(err));
+ rules->dir, virStrerror(err, ebuf, sizeof ebuf));
return;
}
if ((err = writeRules(rules->path, rules->rules, rules->nrules))) {
qemudLog(QEMUD_WARN, _("Failed to saves iptables rules to %s : %s"),
- rules->path, strerror(err));
+ rules->path, virStrerror(err, ebuf, sizeof ebuf));
return;
}
@@ -540,6 +544,7 @@ static void
iptRulesReload(iptRules *rules)
{
int i;
+ char ebuf[1024];
for (i = 0; i < rules->nrules; i++) {
iptRule *rule = &rules->rules[i];
@@ -552,7 +557,8 @@ iptRulesReload(iptRules *rules)
qemudLog(QEMUD_WARN,
_("Failed to remove iptables rule '%s'"
" from chain '%s' in table '%s': %s"),
- rule->rule, rules->chain, rules->table, strerror(errno));
+ rule->rule, rules->chain, rules->table,
+ virStrerror(errno, ebuf, sizeof ebuf));
rule->argv[rule->command_idx] = orig;
}
@@ -561,7 +567,8 @@ iptRulesReload(iptRules *rules)
if (virRun(NULL, rules->rules[i].argv, NULL) < 0)
qemudLog(QEMUD_WARN, _("Failed to add iptables rule '%s'"
" to chain '%s' in table '%s':
%s"),
- rules->rules[i].rule, rules->chain, rules->table,
strerror(errno));
+ rules->rules[i].rule, rules->chain, rules->table,
+ virStrerror(errno, ebuf, sizeof ebuf));
}
/**
diff --git a/src/virterror.c b/src/virterror.c
index 160fea3..6c479f4 100644
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -1005,7 +1005,7 @@ void virReportErrorHelper(virConnectPtr conn, int domcode, int
errcode,
}
-static const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
+const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
{
#ifdef HAVE_STRERROR_R
# ifdef __USE_GNU
diff --git a/src/virterror_internal.h b/src/virterror_internal.h
index 3a11705..709c8ec 100644
--- a/src/virterror_internal.h
+++ b/src/virterror_internal.h
@@ -1,7 +1,7 @@
/*
* virterror.h: internal error handling
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -81,5 +81,6 @@ void virReportOOMErrorFull(virConnectPtr conn,
void virSetGlobalError(void);
void virSetConnError(virConnectPtr conn);
+const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen);
#endif
--
1.6.1.2.418.gd79e6
7:57 a.m.
New subject: [libvirt] [PATCH 4/9] iptables.c: Use virStrerror, not strerror.
On Mon, Feb 02, 2009 at 06:08:17PM +0100, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
* src/iptables.c: Include "virterror_internal.h".
Use virStrerror, not strerror.
* src/virterror.c (virStrerror): Remove "static".
* src/virterror_internal.h (virStrerror): Declare it.
---
src/iptables.c | 25 ++++++++++++++++---------
src/virterror.c | 2 +-
src/virterror_internal.h | 3 ++-
3 files changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/virterror.c b/src/virterror.c
index 160fea3..6c479f4 100644
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -1005,7 +1005,7 @@ void virReportErrorHelper(virConnectPtr conn, int domcode, int
errcode,
}
-static const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
+const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
{
#ifdef HAVE_STRERROR_R
# ifdef __USE_GNU
diff --git a/src/virterror_internal.h b/src/virterror_internal.h
index 3a11705..709c8ec 100644
--- a/src/virterror_internal.h
+++ b/src/virterror_internal.h
@@ -1,7 +1,7 @@
/*
* virterror.h: internal error handling
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -81,5 +81,6 @@ void virReportOOMErrorFull(virConnectPtr conn,
void virSetGlobalError(void);
void virSetConnError(virConnectPtr conn);
+const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen);
This seems to be missing ta change to libvirt_private.sym to actually
export the new function to individual drivers.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
6:07 a.m.
New subject: [libvirt] [PATCH 4/9] iptables.c: Use virStrerror, not strerror.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Mon, Feb 02, 2009 at 06:08:17PM +0100, Jim Meyering wrote:
> From: Jim Meyering <meyering(a)redhat.com>
> * src/iptables.c: Include "virterror_internal.h".
> Use virStrerror, not strerror.
> * src/virterror.c (virStrerror): Remove "static".
> * src/virterror_internal.h (virStrerror): Declare it.
...
> -static const char *virStrerror(int theerrno, char *errBuf,
size_t errBufLen)
> +const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
...
> +const char *virStrerror(int theerrno, char *errBuf, size_t
errBufLen);
This seems to be missing ta change to libvirt_private.sym to
actually
export the new function to individual drivers.
Good catch.
I didn't notice because currently it isn't needed.
I've fixed that and addressed all of your other comments.
Here's an incremental diff that also includes additional changes, e.g.,
- enable the syntax-check for strerror
- change more "out of error" reports to use virReportOOMError
- use QEMUD_ERROR consistently
After that, I've included the full, revised sequence of changes.
Also, not apparent in this incremental, I moved some changes between
change sets and split some, so now there are 10 change sets. I confirmed
that things build after each, just in case via this:
b=034-remove-strerror
git checkout $b
git checkout -b tmp $b
for rev in $(git rev-list HEAD ^master|tac); do
git reset --hard $rev
git log -1
make syntax-check > makerr.sc 2>&1 || break
make check > makerr.c 2>&1 || break
done
diff --git a/Makefile.nonreentrant b/Makefile.nonreentrant
index 13fa59d..b567f31 100644
--- a/Makefile.nonreentrant
+++ b/Makefile.nonreentrant
@@ -79,7 +79,7 @@ NON_REENTRANT += setstate
NON_REENTRANT += sgetspent
NON_REENTRANT += srand48
NON_REENTRANT += srandom
-# NON_REENTRANT += strerror
+NON_REENTRANT += strerror
NON_REENTRANT += strtok
NON_REENTRANT += tmpnam
NON_REENTRANT += ttyname
diff --git a/qemud/Makefile.am b/qemud/Makefile.am
index 372b931..a0c161a 100644
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -107,7 +107,6 @@ libvirtd_LDADD = \
if ! WITH_DRIVER_MODULES
if WITH_QEMU
libvirtd_LDADD += ../src/libvirt_driver_qemu.la
-libvirtd_LDADD += ../src/libvirt_util.la
endif
if WITH_LXC
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a6f4f3e..4338da7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -292,6 +292,7 @@ virEnumToString;
virEventAddHandle;
virEventRemoveHandle;
virExec;
+virSetCloseExec;
virSetNonBlock;
virFormatMacAddr;
virGetHostname;
@@ -326,6 +327,7 @@ virErrorMsg;
virRaiseError;
virReportSystemErrorFull;
virReportOOMErrorFull;
+virStrerror;
# xml.h
diff --git a/src/network_driver.c b/src/network_driver.c
index 8d7340e..4138939 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -904,16 +904,17 @@ static int networkStartNetworkDaemon(virConnectPtr conn,
err_delbr2:
networkRemoveIptablesRules(driver, network);
- err_delbr1:;
- char ebuf[1024];
+ err_delbr1:
if (network->def->ipAddress &&
(err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) {
+ char ebuf[1024];
networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' :
%s\n"),
network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
err_delbr:
if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) {
+ char ebuf[1024];
networkLog(NETWORK_WARN, _("Failed to delete bridge '%s' :
%s\n"),
network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index c331087..a322486 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -218,7 +218,7 @@ qemudAutostartConfigs(struct qemud_driver *driver) {
int ret = qemudStartVMDaemon(conn, driver, vm, NULL, -1);
if (ret < 0) {
virErrorPtr err = virGetLastError();
- qemudLog(QEMUD_ERR, _("Failed to autostart VM '%s':
%s\n"),
+ qemudLog(QEMUD_ERROR, _("Failed to autostart VM '%s':
%s\n"),
vm->def->name,
err ? err->message : NULL);
} else {
@@ -304,7 +304,7 @@ qemudReconnectVMs(struct qemud_driver *driver)
if ((config = virDomainConfigFile(NULL,
driver->stateDir,
vm->def->name)) == NULL) {
- qemudLog(QEMUD_ERR, _("Failed to read domain status for %s\n"),
+ qemudLog(QEMUD_ERROR, _("Failed to read domain status for %s\n"),
vm->def->name);
goto next_error;
}
@@ -314,13 +314,13 @@ qemudReconnectVMs(struct qemud_driver *driver)
vm->newDef = vm->def;
vm->def = status->def;
} else {
- qemudLog(QEMUD_ERR, _("Failed to parse domain status for %s\n"),
+ qemudLog(QEMUD_ERROR, _("Failed to parse domain status for %s\n"),
vm->def->name);
goto next_error;
}
if ((rc = qemudOpenMonitor(NULL, driver, vm, status->monitorpath, 1)) != 0) {
- qemudLog(QEMUD_ERR, _("Failed to reconnect monitor for %s: %d\n"),
+ qemudLog(QEMUD_ERROR, _("Failed to reconnect monitor for %s:
%d\n"),
vm->def->name, rc);
goto next_error;
}
@@ -419,9 +419,9 @@ qemudStartup(void) {
}
if (virFileMakePath(qemu_driver->stateDir) < 0) {
- virReportSystemError(NULL, errno,
- _("Failed to create state dir '%s'"),
- qemu_driver->stateDir);
+ char ebuf[1024];
+ qemudLog(QEMUD_ERROR, _("Failed to create state dir '%s':
%s\n"),
+ qemu_driver->stateDir, virStrerror(errno, ebuf, sizeof ebuf));
goto error;
}
@@ -844,8 +844,11 @@ static int qemudWaitForMonitor(virConnectPtr conn,
ret = qemudReadMonitorOutput(conn, vm, logfd, buf, sizeof(buf),
qemudFindCharDevicePTYs,
"console", 3000);
- if (close(logfd) < 0)
- virReportSystemError(NULL, errno, "%s", _("Unable to close
logfile"));
+ if (close(logfd) < 0) {
+ char ebuf[1024];
+ qemudLog(QEMUD_WARN, _("Unable to close logfile: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
+ }
if (ret == 1) /* Success */
return 0;
@@ -1097,6 +1100,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
const char *emulator;
pid_t child;
int pos = -1;
+ char ebuf[1024];
FD_ZERO(&keepfd);
@@ -1167,30 +1171,30 @@ static int qemudStartVMDaemon(virConnectPtr conn,
tmp = progenv;
while (*tmp) {
if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
- virReportSystemError(NULL, errno,
- "%s", _("Unable to write envv to
logfile"));
+ qemudLog(QEMUD_WARN, _("Unable to write envv to logfile: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
if (safewrite(vm->logfile, " ", 1) < 0)
- virReportSystemError(NULL, errno,
- "%s", _("Unable to write envv to
logfile"));
+ qemudLog(QEMUD_WARN, _("Unable to write envv to logfile: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
- virReportSystemError(NULL, errno,
- "%s", _("Unable to write argv to
logfile"));
+ qemudLog(QEMUD_WARN, _("Unable to write argv to logfile: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
if (safewrite(vm->logfile, " ", 1) < 0)
- virReportSystemError(NULL, errno,
- "%s", _("Unable to write argv to
logfile"));
+ qemudLog(QEMUD_WARN, _("Unable to write argv to logfile: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
tmp++;
}
if (safewrite(vm->logfile, "\n", 1) < 0)
- virReportSystemError(NULL, errno,
- "%s", _("Unable to write argv to
logfile"));
+ qemudLog(QEMUD_WARN, _("Unable to write argv to logfile: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
if ((pos = lseek(vm->logfile, 0, SEEK_END)) < 0)
- virReportSystemError(NULL, errno,
- "%s", _("Unable to seek to end of
logfile"));
+ qemudLog(QEMUD_WARN, _("Unable to seek to end of logfile: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
for (i = 0 ; i < ntapfds ; i++)
FD_SET(tapfds[i], &keepfd);
@@ -1277,8 +1281,11 @@ static void qemudShutdownVMDaemon(virConnectPtr conn
ATTRIBUTE_UNUSED,
vm->monitor_watch = -1;
}
- if (close(vm->logfile) < 0)
- virReportSystemError(conn, errno, "%s", _("Unable to close
logfile"));
+ if (close(vm->logfile) < 0) {
+ char ebuf[1024];
+ qemudLog(QEMUD_WARN, _("Unable to close logfile: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
+ }
if (vm->monitor != -1)
close(vm->monitor);
vm->logfile = -1;
@@ -1444,9 +1451,11 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm,
}
/* Log, but ignore failures to write logfile for VM */
- if (safewrite(vm->logfile, buf, strlen(buf)) < 0)
- virReportSystemError(NULL, errno,
- "%s", _("Unable to log VM console
data"));
+ if (safewrite(vm->logfile, buf, strlen(buf)) < 0) {
+ char ebuf[1024];
+ qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
+ }
*reply = buf;
return 0;
@@ -1454,9 +1463,11 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm,
error:
if (buf) {
/* Log, but ignore failures to write logfile for VM */
- if (safewrite(vm->logfile, buf, strlen(buf)) < 0)
- virReportSystemError(NULL, errno,
- "%s", _("Unable to log VM console
data"));
+ if (safewrite(vm->logfile, buf, strlen(buf)) < 0) {
+ char ebuf[1024];
+ qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
+ }
VIR_FREE(buf);
}
return -1;
@@ -1562,7 +1573,7 @@ static int kvmGetMaxVCPUs(void) {
fd = open(KVM_DEVICE, O_RDONLY);
if (fd < 0) {
virReportSystemError(NULL, errno, _("Unable to open %s"), KVM_DEVICE);
- return maxvcpus;
+ return -1;
}
r = ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
@@ -2407,15 +2418,13 @@ static int qemudDomainSave(virDomainPtr dom,
/* Migrate to file */
safe_path = qemudEscapeShellArg(path);
if (!safe_path) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(dom->conn);
goto cleanup;
}
if (virAsprintf(&command, "migrate \"exec:"
"dd of='%s' oflag=append conv=notrunc
2>/dev/null"
"\"", safe_path) == -1) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(dom->conn);
command = NULL;
goto cleanup;
}
@@ -2729,8 +2738,7 @@ static int qemudDomainRestore(virConnectPtr conn,
}
if (VIR_ALLOC_N(xml, header.xml_len) < 0) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(conn);
goto cleanup;
}
@@ -3208,8 +3216,7 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
safe_path = qemudEscapeMonitorArg(dev->data.disk->src);
if (!safe_path) {
- qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(conn);
return -1;
}
@@ -3278,8 +3285,7 @@ static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr
conn,
safe_path = qemudEscapeMonitorArg(dev->data.disk->src);
if (!safe_path) {
- qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(conn);
return -1;
}
diff --git a/src/storage_driver.c b/src/storage_driver.c
index d644b63..f1320c5 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -567,10 +567,11 @@ storagePoolUndefine(virStoragePoolPtr obj) {
if (virStoragePoolObjDeleteDef(obj->conn, pool) < 0)
goto cleanup;
- char ebuf[1024];
- if (unlink(pool->autostartLink) < 0 && errno != ENOENT && errno
!= ENOTDIR)
+ if (unlink(pool->autostartLink) < 0 && errno != ENOENT && errno
!= ENOTDIR) {
+ char ebuf[1024];
storageLog("Failed to delete autostart link '%s': %s",
pool->autostartLink, virStrerror(errno, ebuf, sizeof ebuf));
+ }
VIR_FREE(pool->configFile);
VIR_FREE(pool->autostartLink);
diff --git a/src/uml_driver.c b/src/uml_driver.c
index 48e2d07..c5a06a2 100644
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -727,6 +727,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
int *tapfds = NULL;
int ntapfds = 0;
fd_set keepfd;
+ char ebuf[1024];
FD_ZERO(&keepfd);
@@ -789,7 +790,6 @@ static int umlStartVMDaemon(virConnectPtr conn,
return -1;
}
- char ebuf[1024];
tmp = progenv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
diff --git a/src/util.c b/src/util.c
index 1034455..01fe37a 100644
--- a/src/util.c
+++ b/src/util.c
@@ -182,6 +182,9 @@ int virSetNonBlock(int fd) {
return 0;
}
+
+#ifndef WIN32
+
int virSetCloseExec(int fd) {
int flags;
if ((flags = fcntl(fd, F_GETFD)) < 0)
@@ -192,8 +195,6 @@ int virSetCloseExec(int fd) {
return 0;
}
-#ifndef WIN32
-
static int
__virExec(virConnectPtr conn,
const char *const*argv,
diff --git a/src/uuid.c b/src/uuid.c
index 9e9416f..6f7d85f 100644
--- a/src/uuid.c
+++ b/src/uuid.c
@@ -100,12 +100,13 @@ virUUIDGenerate(unsigned char *uuid)
if (uuid == NULL)
return(-1);
- char ebuf[1024];
- if ((err = virUUIDGenerateRandomBytes(uuid, VIR_UUID_BUFLEN)))
+ if ((err = virUUIDGenerateRandomBytes(uuid, VIR_UUID_BUFLEN))) {
+ char ebuf[1024];
qemudLog(QEMUD_WARN,
_("Falling back to pseudorandom UUID,"
" failed to generate random bytes: %s"),
virStrerror(err, ebuf, sizeof ebuf));
+ }
return virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN);
}
Here's the 10-change-set sequence:
7:22 a.m.
New subject: [libvirt] [PATCH 4/9] iptables.c: Use virStrerror, not strerror.
On Wed, Feb 04, 2009 at 01:07:49PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> On Mon, Feb 02, 2009 at 06:08:17PM +0100, Jim Meyering wrote:
>> From: Jim Meyering <meyering(a)redhat.com>
>> * src/iptables.c: Include "virterror_internal.h".
>> Use virStrerror, not strerror.
>> * src/virterror.c (virStrerror): Remove "static".
>> * src/virterror_internal.h (virStrerror): Declare it.
...
>> -static const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
>> +const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
...
>> +const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen);
> This seems to be missing ta change to libvirt_private.sym to actually
> export the new function to individual drivers.
Good catch.
I didn't notice because currently it isn't needed.
I've fixed that and addressed all of your other comments.
Here's an incremental diff that also includes additional changes, e.g.,
- enable the syntax-check for strerror
- change more "out of error" reports to use virReportOOMError
- use QEMUD_ERROR consistently
diff --git a/Makefile.nonreentrant b/Makefile.nonreentrant
index 13fa59d..b567f31 100644
--- a/Makefile.nonreentrant
+++ b/Makefile.nonreentrant
@@ -79,7 +79,7 @@ NON_REENTRANT += setstate
NON_REENTRANT += sgetspent
NON_REENTRANT += srand48
NON_REENTRANT += srandom
-# NON_REENTRANT += strerror
+NON_REENTRANT += strerror
NON_REENTRANT += strtok
NON_REENTRANT += tmpnam
NON_REENTRANT += ttyname
diff --git a/qemud/Makefile.am b/qemud/Makefile.am
index 372b931..a0c161a 100644
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -107,7 +107,6 @@ libvirtd_LDADD = \
if ! WITH_DRIVER_MODULES
if WITH_QEMU
libvirtd_LDADD += ../src/libvirt_driver_qemu.la
-libvirtd_LDADD += ../src/libvirt_util.la
endif
if WITH_LXC
Someting seems not right here - we shouldn't need this change,
since this shouldn't be added in the first place.
diff --git a/src/network_driver.c b/src/network_driver.c
index 8d7340e..4138939 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -904,16 +904,17 @@ static int networkStartNetworkDaemon(virConnectPtr conn,
err_delbr2:
networkRemoveIptablesRules(driver, network);
- err_delbr1:;
- char ebuf[1024];
+ err_delbr1:
if (network->def->ipAddress &&
(err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) {
+ char ebuf[1024];
networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' :
%s\n"),
network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
This also appears to be changing code which doesn't exist.
[snip more of the same]
Have you got the actual patch against current CVS, since this doesn't
appear to be it ?
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
7:30 a.m.
New subject: [libvirt] [PATCH 4/9] iptables.c: Use virStrerror, not strerror.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Wed, Feb 04, 2009 at 01:07:49PM +0100, Jim Meyering wrote:
...
> Here's an incremental diff that also includes additional
changes, e.g.,
^^^^^^^^^^^^^^^^
> - enable the syntax-check for strerror
> - change more "out of error" reports to use virReportOOMError
> - use QEMUD_ERROR consistently
...
> diff --git a/Makefile.nonreentrant b/Makefile.nonreentrant
> index 13fa59d..b567f31 100644
> --- a/Makefile.nonreentrant
> +++ b/Makefile.nonreentrant
> @@ -79,7 +79,7 @@ NON_REENTRANT += setstate
> NON_REENTRANT += sgetspent
> NON_REENTRANT += srand48
> NON_REENTRANT += srandom
> -# NON_REENTRANT += strerror
> +NON_REENTRANT += strerror
> NON_REENTRANT += strtok
> NON_REENTRANT += tmpnam
> NON_REENTRANT += ttyname
> diff --git a/qemud/Makefile.am b/qemud/Makefile.am
> index 372b931..a0c161a 100644
> --- a/qemud/Makefile.am
> +++ b/qemud/Makefile.am
> @@ -107,7 +107,6 @@ libvirtd_LDADD = \
> if ! WITH_DRIVER_MODULES
> if WITH_QEMU
> libvirtd_LDADD += ../src/libvirt_driver_qemu.la
> -libvirtd_LDADD += ../src/libvirt_util.la
> endif
>
> if WITH_LXC
Someting seems not right here - we shouldn't need this change,
since this shouldn't be added in the first place.
That's the *incremental* change, against what you reviewed the
first time.
> diff --git a/src/network_driver.c b/src/network_driver.c
> index 8d7340e..4138939 100644
> --- a/src/network_driver.c
> +++ b/src/network_driver.c
> @@ -904,16 +904,17 @@ static int networkStartNetworkDaemon(virConnectPtr conn,
> err_delbr2:
> networkRemoveIptablesRules(driver, network);
>
> - err_delbr1:;
> - char ebuf[1024];
> + err_delbr1:
> if (network->def->ipAddress &&
> (err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0)))
{
> + char ebuf[1024];
> networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' :
%s\n"),
> network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
> }
This also appears to be changing code which doesn't exist.
Same as above.
[snip more of the same]
Have you got the actual patch against current CVS, since this doesn't
appear to be it ?
Yep. It was attached at the end, after this:
> Here's the 10-change-set sequence:
7:41 a.m.
New subject: [libvirt] [PATCH 4/9] iptables.c: Use virStrerror, not strerror.
On Thu, Feb 05, 2009 at 02:30:02PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> On Wed, Feb 04, 2009 at 01:07:49PM +0100, Jim Meyering wrote:
...
>> Here's an incremental diff that also includes additional changes, e.g.,
^^^^^^^^^^^^^^^^
> [snip more of the same]
>
> Have you got the actual patch against current CVS, since this doesn't
> appear to be it ?
Yep. It was attached at the end, after this:
>> Here's the 10-change-set sequence:
This is the section I was quoting - it a seems to be identical to the
incremental diff.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
7:49 a.m.
New subject: [libvirt] [PATCH 4/9] iptables.c: Use virStrerror, not strerror.
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Thu, Feb 05, 2009 at 02:30:02PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> > On Wed, Feb 04, 2009 at 01:07:49PM +0100, Jim Meyering wrote:
> ...
> >> Here's an incremental diff that also includes additional changes, e.g.,
> ^^^^^^^^^^^^^^^^
> > [snip more of the same]
> >
> > Have you got the actual patch against current CVS, since this doesn't
> > appear to be it ?
>
> Yep. It was attached at the end, after this:
>
> >> Here's the 10-change-set sequence:
This is the section I was quoting - it a seems to be identical to the
incremental diff.
Oh. So it is. Sorry about that.
Here's the full diff:
From 51de11e2ec42e65b756d714afe94b02cd27e38e9 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Tue, 3 Feb 2009 21:42:39 +0100
Subject: [PATCH 01/10] publicize virStrerror
* src/virterror.c (virStrerror): Remove "static".
* src/virterror_internal.h (virStrerror): Declare it.
* src/libvirt_private.syms: Add virStrerror;
---
src/libvirt_private.syms | 1 +
src/virterror.c | 2 +-
src/virterror_internal.h | 3 ++-
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index a6f4f3e..17aefbe 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -326,6 +326,7 @@ virErrorMsg;
virRaiseError;
virReportSystemErrorFull;
virReportOOMErrorFull;
+virStrerror;
# xml.h
diff --git a/src/virterror.c b/src/virterror.c
index 160fea3..6c479f4 100644
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -1005,7 +1005,7 @@ void virReportErrorHelper(virConnectPtr conn, int domcode, int
errcode,
}
-static const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
+const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen)
{
#ifdef HAVE_STRERROR_R
# ifdef __USE_GNU
diff --git a/src/virterror_internal.h b/src/virterror_internal.h
index 3a11705..709c8ec 100644
--- a/src/virterror_internal.h
+++ b/src/virterror_internal.h
@@ -1,7 +1,7 @@
/*
* virterror.h: internal error handling
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -81,5 +81,6 @@ void virReportOOMErrorFull(virConnectPtr conn,
void virSetGlobalError(void);
void virSetConnError(virConnectPtr conn);
+const char *virStrerror(int theerrno, char *errBuf, size_t errBufLen);
#endif
--
1.6.1.2.511.gc5d3f
From a5cf1250b325ae2d1fa62b45bcdfff47c97da035 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Wed, 28 Jan 2009 19:20:08 +0100
Subject: [PATCH 02/10] qemu_driver.c: use virReportSystemError in place of some qemudLog
uses
...thus eliminating many uses of strerror.
(kvmGetMaxVCPUs): Rather than merely warning, diagnose an error
and return -1 upon failure to open KVM_DEVICE. Suggested by
Daniel P. Berrange.
---
src/qemu_driver.c | 97 +++++++++++++++++++++++++++--------------------------
1 files changed, 49 insertions(+), 48 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index ebcdd88..3a790c1 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -207,22 +207,21 @@ qemudLogReadFD(virConnectPtr conn, const char* logDir, const char*
name, off_t p
if ((fd = open(logfile, logmode)) < 0) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("failed to create logfile %s: %s"),
- logfile, strerror(errno));
+ virReportSystemError(conn, errno,
+ _("failed to create logfile %s"),
+ logfile);
return -1;
}
if (qemudSetCloseExec(fd) < 0) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("Unable to set VM logfile close-on-exec flag: %s"),
- strerror(errno));
+ virReportSystemError(conn, errno, "%s",
+ _("Unable to set VM logfile close-on-exec
flag"));
close(fd);
return -1;
}
if (lseek(fd, pos, SEEK_SET) < 0) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _("Unable to seek to %lld in %s: %s"),
- (long long) pos, logfile, strerror(errno));
+ virReportSystemError(conn, errno,
+ _("Unable to seek to %lld in %s"),
+ (long long) pos, logfile);
close(fd);
}
return fd;
@@ -250,7 +249,7 @@ qemudAutostartConfigs(struct qemud_driver *driver) {
int ret = qemudStartVMDaemon(conn, driver, vm, NULL, -1);
if (ret < 0) {
virErrorPtr err = virGetLastError();
- qemudLog(QEMUD_ERR, _("Failed to autostart VM '%s':
%s\n"),
+ qemudLog(QEMUD_ERROR, _("Failed to autostart VM '%s':
%s\n"),
vm->def->name,
err ? err->message : NULL);
} else {
@@ -336,7 +335,7 @@ qemudReconnectVMs(struct qemud_driver *driver)
if ((config = virDomainConfigFile(NULL,
driver->stateDir,
vm->def->name)) == NULL) {
- qemudLog(QEMUD_ERR, _("Failed to read domain status for %s\n"),
+ qemudLog(QEMUD_ERROR, _("Failed to read domain status for %s\n"),
vm->def->name);
goto next_error;
}
@@ -346,13 +345,13 @@ qemudReconnectVMs(struct qemud_driver *driver)
vm->newDef = vm->def;
vm->def = status->def;
} else {
- qemudLog(QEMUD_ERR, _("Failed to parse domain status for %s\n"),
+ qemudLog(QEMUD_ERROR, _("Failed to parse domain status for %s\n"),
vm->def->name);
goto next_error;
}
if ((rc = qemudOpenMonitor(NULL, driver, vm, status->monitorpath, 1)) != 0) {
- qemudLog(QEMUD_ERR, _("Failed to reconnect monitor for %s: %d\n"),
+ qemudLog(QEMUD_ERROR, _("Failed to reconnect monitor for %s:
%d\n"),
vm->def->name, rc);
goto next_error;
}
@@ -451,7 +450,7 @@ qemudStartup(void) {
}
if (virFileMakePath(qemu_driver->stateDir) < 0) {
- qemudLog(QEMUD_ERR, _("Failed to create state dir '%s':
%s\n"),
+ qemudLog(QEMUD_ERROR, _("Failed to create state dir '%s':
%s\n"),
qemu_driver->stateDir, strerror(errno));
goto error;
}
@@ -493,7 +492,7 @@ qemudStartup(void) {
return 0;
out_of_memory:
- qemudLog (QEMUD_ERR,
+ qemudLog (QEMUD_ERROR,
"%s", _("qemudStartup: out of memory\n"));
error:
if (qemu_driver)
@@ -1200,30 +1199,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));
+ qemudLog(QEMUD_WARN, _("Unable to write envv to logfile: %s\n"),
+ strerror(errno));
if (safewrite(vm->logfile, " ", 1) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write envv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ qemudLog(QEMUD_WARN, _("Unable to write envv to logfile: %s\n"),
+ strerror(errno));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ qemudLog(QEMUD_WARN, _("Unable to write argv to logfile: %s\n"),
+ strerror(errno));
if (safewrite(vm->logfile, " ", 1) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ qemudLog(QEMUD_WARN, _("Unable to write argv to logfile: %s\n"),
+ strerror(errno));
tmp++;
}
if (safewrite(vm->logfile, "\n", 1) < 0)
- qemudLog(QEMUD_WARN, _("Unable to write argv to logfile %d: %s\n"),
- errno, strerror(errno));
+ qemudLog(QEMUD_WARN, _("Unable to write argv to logfile: %s\n"),
+ strerror(errno));
if ((pos = lseek(vm->logfile, 0, SEEK_END)) < 0)
- qemudLog(QEMUD_WARN, _("Unable to seek to end of logfile %d: %s\n"),
- errno, strerror(errno));
+ qemudLog(QEMUD_WARN, _("Unable to seek to end of logfile: %s\n"),
+ strerror(errno));
for (i = 0 ; i < ntapfds ; i++)
FD_SET(tapfds[i], &keepfd);
@@ -1292,7 +1291,8 @@ static int qemudStartVMDaemon(virConnectPtr conn,
static void qemudShutdownVMDaemon(virConnectPtr conn ATTRIBUTE_UNUSED,
- struct qemud_driver *driver, virDomainObjPtr vm) {
+ struct qemud_driver *driver,
+ virDomainObjPtr vm) {
if (!virDomainIsActive(vm))
return;
@@ -1300,8 +1300,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);
if (vm->monitor_watch != -1) {
virEventRemoveHandle(vm->monitor_watch);
@@ -1309,8 +1310,8 @@ static void qemudShutdownVMDaemon(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
if (close(vm->logfile) < 0)
- qemudLog(QEMUD_WARN, _("Unable to close logfile %d: %s\n"),
- errno, strerror(errno));
+ qemudLog(QEMUD_WARN, _("Unable to close logfile: %s\n"),
+ strerror(errno));
if (vm->monitor != -1)
close(vm->monitor);
vm->logfile = -1;
@@ -1593,8 +1594,8 @@ 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));
- return maxvcpus;
+ virReportSystemError(NULL, errno, _("Unable to open %s"), KVM_DEVICE);
+ return -1;
}
r = ioctl(fd, KVM_CHECK_EXTENSION, KVM_CAP_NR_VCPUS);
@@ -1836,8 +1837,8 @@ qemudGetHostname (virConnectPtr conn)
result = virGetHostname();
if (result == NULL) {
- qemudReportError (conn, NULL, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (conn, errno,
+ "%s", _("failed to determine host
name"));
return NULL;
}
/* Caller frees this string. */
@@ -3922,8 +3923,8 @@ qemudDomainBlockPeek (virDomainPtr dom,
/* The path is correct, now try to open it and get its size. */
fd = open (path, O_RDONLY);
if (fd == -1) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dom->conn, errno,
+ _("%s: failed to open"), path);
goto cleanup;
}
@@ -3933,8 +3934,8 @@ qemudDomainBlockPeek (virDomainPtr dom,
*/
if (lseek (fd, offset, SEEK_SET) == (off_t) -1 ||
saferead (fd, buffer, size) == (ssize_t) -1) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dom->conn, errno,
+ _("%s: failed to seek or read"), path);
goto cleanup;
}
@@ -3988,8 +3989,8 @@ qemudDomainMemoryPeek (virDomainPtr dom,
/* Create a temporary filename. */
if ((fd = mkstemp (tmp)) == -1) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dom->conn, errno,
+ _("mkstemp(\"%s\") failed"), tmp);
goto cleanup;
}
@@ -4005,8 +4006,9 @@ qemudDomainMemoryPeek (virDomainPtr dom,
/* Read the memory file into buffer. */
if (saferead (fd, buffer, size) == (ssize_t) -1) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dom->conn, errno,
+ _("failed to read temporary file "
+ "created with template %s"), tmp);
goto cleanup;
}
@@ -4165,8 +4167,8 @@ qemudDomainMigratePrepare2 (virConnectPtr dconn,
/* Get hostname */
if (gethostname (hostname, HOST_NAME_MAX+1) == -1) {
- qemudReportError (dconn, NULL, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportSystemError (dconn, errno,
+ "%s", _("failed to determine host
name"));
goto cleanup;
}
@@ -4336,8 +4338,7 @@ qemudDomainMigratePerform (virDomainPtr dom,
/* Issue the migrate command. */
safe_uri = qemudEscapeMonitorArg (uri);
if (!safe_uri) {
- qemudReportError (dom->conn, dom, NULL, VIR_ERR_SYSTEM_ERROR,
- "%s", strerror (errno));
+ virReportOOMError (dom->conn);
goto cleanup;
}
snprintf (cmd, sizeof cmd, "migrate \"%s\"", safe_uri);
--
1.6.1.2.511.gc5d3f
From dc64e55aa9f1dc7700d83bf767b436b2980f10f4 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 30 Jan 2009 21:37:20 +0100
Subject: [PATCH 03/10] remove duplicate *SetCloseExec and *SetNonBlock functions
* src/qemu_driver.c: Use virSetCloseExec and virSetNonBlock,
rather than qemuSet* functions. Suggested by Daniel P. Berrange.
* src/util.c (virSetCloseExec): Publicize.
* src/util.h (virSetCloseExec): Declare
* src/libvirt_private.syms: Add virSetCloseExec.
---
src/libvirt_private.syms | 1 +
src/qemu_driver.c | 39 ++++-----------------------------------
src/util.c | 4 ++--
src/util.h | 1 +
4 files changed, 8 insertions(+), 37 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 17aefbe..4338da7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -292,6 +292,7 @@ virEnumToString;
virEventAddHandle;
virEventRemoveHandle;
virExec;
+virSetCloseExec;
virSetNonBlock;
virFormatMacAddr;
virGetHostname;
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 3a790c1..8d8d711 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -91,37 +91,6 @@ static void qemuDriverUnlock(struct qemud_driver *driver)
virMutexUnlock(&driver->lock);
}
-static int qemudSetCloseExec(int fd) {
- int flags;
- if ((flags = fcntl(fd, F_GETFD)) < 0)
- goto error;
- flags |= FD_CLOEXEC;
- if ((fcntl(fd, F_SETFD, flags)) < 0)
- goto error;
- return 0;
- error:
- qemudLog(QEMUD_ERR,
- "%s", _("Failed to set close-on-exec file descriptor
flag\n"));
- return -1;
-}
-
-
-static int qemudSetNonBlock(int fd) {
- int flags;
- if ((flags = fcntl(fd, F_GETFL)) < 0)
- goto error;
- flags |= O_NONBLOCK;
- if ((fcntl(fd, F_SETFL, flags)) < 0)
- goto error;
- return 0;
- error:
- qemudLog(QEMUD_ERR,
- "%s", _("Failed to set non-blocking file descriptor
flag\n"));
- return -1;
-}
-
-
-
static void qemuDomainEventFlush(int timer, void *opaque);
static void qemuDomainEventQueue(struct qemud_driver *driver,
virDomainEventPtr event);
@@ -180,7 +149,7 @@ qemudLogFD(virConnectPtr conn, const char* logDir, const char* name)
logfile);
return -1;
}
- if (qemudSetCloseExec(fd) < 0) {
+ if (virSetCloseExec(fd) < 0) {
virReportSystemError(conn, errno, "%s",
_("Unable to set VM logfile close-on-exec
flag"));
close(fd);
@@ -212,7 +181,7 @@ qemudLogReadFD(virConnectPtr conn, const char* logDir, const char*
name, off_t p
logfile);
return -1;
}
- if (qemudSetCloseExec(fd) < 0) {
+ if (virSetCloseExec(fd) < 0) {
virReportSystemError(conn, errno, "%s",
_("Unable to set VM logfile close-on-exec
flag"));
close(fd);
@@ -721,12 +690,12 @@ static int qemudOpenMonitor(virConnectPtr conn,
_("Unable to open monitor path %s"), monitor);
return -1;
}
- if (qemudSetCloseExec(monfd) < 0) {
+ if (virSetCloseExec(monfd) < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Unable to set monitor close-on-exec
flag"));
goto error;
}
- if (qemudSetNonBlock(monfd) < 0) {
+ if (virSetNonBlock(monfd) < 0) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
"%s", _("Unable to put monitor into non-blocking
mode"));
goto error;
diff --git a/src/util.c b/src/util.c
index 7c7fe31..96c1b00 100644
--- a/src/util.c
+++ b/src/util.c
@@ -1,7 +1,7 @@
/*
* utils.c: common, generic utility functions
*
- * Copyright (C) 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
* Copyright (C) 2006, 2007 Binary Karma
* Copyright (C) 2006 Shuveb Hussain
@@ -185,7 +185,7 @@ int virSetNonBlock(int fd) {
#ifndef WIN32
-static int virSetCloseExec(int fd) {
+int virSetCloseExec(int fd) {
int flags;
if ((flags = fcntl(fd, F_GETFD)) < 0)
return -1;
diff --git a/src/util.h b/src/util.h
index c553264..4667b92 100644
--- a/src/util.h
+++ b/src/util.h
@@ -39,6 +39,7 @@ enum {
};
int virSetNonBlock(int fd);
+int virSetCloseExec(int fd);
int virExec(virConnectPtr conn,
const char *const*argv,
--
1.6.1.2.511.gc5d3f
From 929ea43e5c36d3e7b44d3855ad8956ba7f5a5d0f Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Tue, 3 Feb 2009 21:51:42 +0100
Subject: [PATCH 04/10] qemu_driver.c: s/strerror/virStrerror.../
Perform most changs automatically, with this:
perl -pi -e 's/\bstrerror *\((.+?)\)/virStrerror($1, ebuf, sizeof ebuf)/'
Then, declare ebuf, as needed
---
src/qemu_driver.c | 40 +++++++++++++++++++++++++---------------
1 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index 8d8d711..da25063 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -419,8 +419,9 @@ qemudStartup(void) {
}
if (virFileMakePath(qemu_driver->stateDir) < 0) {
+ char ebuf[1024];
qemudLog(QEMUD_ERROR, _("Failed to create state dir '%s':
%s\n"),
- qemu_driver->stateDir, strerror(errno));
+ qemu_driver->stateDir, virStrerror(errno, ebuf, sizeof ebuf));
goto error;
}
@@ -844,9 +845,11 @@ static int qemudWaitForMonitor(virConnectPtr conn,
ret = qemudReadMonitorOutput(conn, vm, logfd, buf, sizeof(buf),
qemudFindCharDevicePTYs,
"console", 3000);
- if (close(logfd) < 0)
+ if (close(logfd) < 0) {
+ char ebuf[1024];
qemudLog(QEMUD_WARN, _("Unable to close logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
+ }
if (ret == 1) /* Success */
return 0;
@@ -1098,6 +1101,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
const char *emulator;
pid_t child;
int pos = -1;
+ char ebuf[1024];
FD_ZERO(&keepfd);
@@ -1169,29 +1173,29 @@ static int qemudStartVMDaemon(virConnectPtr conn,
while (*tmp) {
if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
qemudLog(QEMUD_WARN, _("Unable to write envv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
if (safewrite(vm->logfile, " ", 1) < 0)
qemudLog(QEMUD_WARN, _("Unable to write envv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(vm->logfile, *tmp, strlen(*tmp)) < 0)
qemudLog(QEMUD_WARN, _("Unable to write argv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
if (safewrite(vm->logfile, " ", 1) < 0)
qemudLog(QEMUD_WARN, _("Unable to write argv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
tmp++;
}
if (safewrite(vm->logfile, "\n", 1) < 0)
qemudLog(QEMUD_WARN, _("Unable to write argv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
if ((pos = lseek(vm->logfile, 0, SEEK_END)) < 0)
qemudLog(QEMUD_WARN, _("Unable to seek to end of logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
for (i = 0 ; i < ntapfds ; i++)
FD_SET(tapfds[i], &keepfd);
@@ -1278,9 +1282,11 @@ static void qemudShutdownVMDaemon(virConnectPtr conn
ATTRIBUTE_UNUSED,
vm->monitor_watch = -1;
}
- if (close(vm->logfile) < 0)
+ if (close(vm->logfile) < 0) {
+ char ebuf[1024];
qemudLog(QEMUD_WARN, _("Unable to close logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
+ }
if (vm->monitor != -1)
close(vm->monitor);
vm->logfile = -1;
@@ -1446,9 +1452,11 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm,
}
/* Log, but ignore failures to write logfile for VM */
- if (safewrite(vm->logfile, buf, strlen(buf)) < 0)
+ if (safewrite(vm->logfile, buf, strlen(buf)) < 0) {
+ char ebuf[1024];
qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
+ }
*reply = buf;
return 0;
@@ -1456,9 +1464,11 @@ qemudMonitorCommandExtra(const virDomainObjPtr vm,
error:
if (buf) {
/* Log, but ignore failures to write logfile for VM */
- if (safewrite(vm->logfile, buf, strlen(buf)) < 0)
+ if (safewrite(vm->logfile, buf, strlen(buf)) < 0) {
+ char ebuf[1024];
qemudLog(QEMUD_WARN, _("Unable to log VM console data: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
+ }
VIR_FREE(buf);
}
return -1;
--
1.6.1.2.511.gc5d3f
From 9fc4162e009f0bfaae1096103c8f980aff4bfbc0 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 30 Jan 2009 23:18:04 +0100
Subject: [PATCH 05/10] use virReportOOMError rather than more verbose equivalent
* src/qemu_driver.c (qemudDomainSave, qemudDomainRestore)
(qemudDomainAttachPciDiskDevice)
(qemudDomainAttachUsbMassstorageDevice):
---
src/qemu_driver.c | 18 ++++++------------
1 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index da25063..a322486 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -462,8 +462,7 @@ qemudStartup(void) {
return 0;
out_of_memory:
- qemudLog (QEMUD_ERROR,
- "%s", _("qemudStartup: out of memory\n"));
+ virReportOOMError(NULL);
error:
if (qemu_driver)
qemuDriverUnlock(qemu_driver);
@@ -2419,15 +2418,13 @@ static int qemudDomainSave(virDomainPtr dom,
/* Migrate to file */
safe_path = qemudEscapeShellArg(path);
if (!safe_path) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(dom->conn);
goto cleanup;
}
if (virAsprintf(&command, "migrate \"exec:"
"dd of='%s' oflag=append conv=notrunc
2>/dev/null"
"\"", safe_path) == -1) {
- qemudReportError(dom->conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(dom->conn);
command = NULL;
goto cleanup;
}
@@ -2741,8 +2738,7 @@ static int qemudDomainRestore(virConnectPtr conn,
}
if (VIR_ALLOC_N(xml, header.xml_len) < 0) {
- qemudReportError(conn, NULL, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(conn);
goto cleanup;
}
@@ -3220,8 +3216,7 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr conn,
safe_path = qemudEscapeMonitorArg(dev->data.disk->src);
if (!safe_path) {
- qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(conn);
return -1;
}
@@ -3290,8 +3285,7 @@ static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr
conn,
safe_path = qemudEscapeMonitorArg(dev->data.disk->src);
if (!safe_path) {
- qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
- "%s", _("out of memory"));
+ virReportOOMError(conn);
return -1;
}
--
1.6.1.2.511.gc5d3f
From eac7054c8e9919c7c4940328902ed5acfae7a260 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 2 Feb 2009 10:03:35 +0100
Subject: [PATCH 06/10] iptables.c: Use virStrerror, not strerror.
* src/iptables.c: Include "virterror_internal.h".
Use virStrerror, not strerror.
* src/iptables.c (notifyRulesUpdated): Use %s rather than
string-concatenation that made sc_unmarked_diagnostics report
a false-positive.
---
src/iptables.c | 23 +++++++++++++++--------
1 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/src/iptables.c b/src/iptables.c
index c850b9e..9011277 100644
--- a/src/iptables.c
+++ b/src/iptables.c
@@ -44,6 +44,7 @@
#include "iptables.h"
#include "util.h"
#include "memory.h"
+#include "virterror_internal.h"
#define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -98,9 +99,10 @@ notifyRulesUpdated(const char *table,
argv[2] = arg;
argv[3] = NULL;
+ char ebuf[1024];
if (virRun(NULL, argv, NULL) < 0)
- qemudLog(QEMUD_WARN, _("Failed to run '" LOKKIT_PATH
- " %s' : %s"), arg, strerror(errno));
+ qemudLog(QEMUD_WARN, _("Failed to run '%s %s': %s"),
+ LOKKIT_PATH, arg, virStrerror(errno, ebuf, sizeof ebuf));
}
static int
@@ -174,10 +176,11 @@ notifyRulesRemoved(const char *table,
return;
- write_error:
+ write_error:;
+ char ebuf[1024];
qemudLog(QEMUD_WARN, _("Failed to write to " SYSCONF_DIR
"/sysconfig/system-config-firewall : %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
if (f)
fclose(f);
VIR_FREE(content);
@@ -239,15 +242,16 @@ iptRulesSave(iptRules *rules)
#ifdef ENABLE_IPTABLES_LOKKIT
int err;
+ char ebuf[1024];
if ((err = virFileMakePath(rules->dir))) {
qemudLog(QEMUD_WARN, _("Failed to create directory %s : %s"),
- rules->dir, strerror(err));
+ rules->dir, virStrerror(err, ebuf, sizeof ebuf));
return;
}
if ((err = writeRules(rules->path, rules->rules, rules->nrules))) {
qemudLog(QEMUD_WARN, _("Failed to saves iptables rules to %s : %s"),
- rules->path, strerror(err));
+ rules->path, virStrerror(err, ebuf, sizeof ebuf));
return;
}
@@ -537,6 +541,7 @@ static void
iptRulesReload(iptRules *rules)
{
int i;
+ char ebuf[1024];
for (i = 0; i < rules->nrules; i++) {
iptRule *rule = &rules->rules[i];
@@ -549,7 +554,8 @@ iptRulesReload(iptRules *rules)
qemudLog(QEMUD_WARN,
_("Failed to remove iptables rule '%s'"
" from chain '%s' in table '%s': %s"),
- rule->rule, rules->chain, rules->table, strerror(errno));
+ rule->rule, rules->chain, rules->table,
+ virStrerror(errno, ebuf, sizeof ebuf));
rule->argv[rule->command_idx] = orig;
}
@@ -558,7 +564,8 @@ iptRulesReload(iptRules *rules)
if (virRun(NULL, rules->rules[i].argv, NULL) < 0)
qemudLog(QEMUD_WARN, _("Failed to add iptables rule '%s'"
" to chain '%s' in table '%s':
%s"),
- rules->rules[i].rule, rules->chain, rules->table,
strerror(errno));
+ rules->rules[i].rule, rules->chain, rules->table,
+ virStrerror(errno, ebuf, sizeof ebuf));
}
/**
--
1.6.1.2.511.gc5d3f
From 04e6663f82a19a138109eab08d85ef56b246780a Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 2 Feb 2009 11:26:33 +0100
Subject: [PATCH 07/10] qemud.c: use virStrerror, not strerror
---
qemud/qemud.c | 65 +++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/qemud/qemud.c b/qemud/qemud.c
index fa5e17d..48083df 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -231,8 +231,9 @@ remoteCheckCertFile(const char *type, const char *file)
{
struct stat sb;
if (stat(file, &sb) < 0) {
- VIR_ERROR(_("Cannot access %s '%s': %s (%d)"),
- type, file, strerror(errno), errno);
+ char ebuf[1024];
+ VIR_ERROR(_("Cannot access %s '%s': %s"),
+ type, file, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
return 0;
@@ -331,7 +332,9 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED,
virMutexLock(&server->lock);
if (saferead(server->sigread, &siginfo, sizeof(siginfo)) != sizeof(siginfo))
{
- VIR_ERROR(_("Failed to read from signal pipe: %s"), strerror(errno));
+ char ebuf[1024];
+ VIR_ERROR(_("Failed to read from signal pipe: %s"),
+ virStrerror(errno, ebuf, sizeof ebuf));
virMutexUnlock(&server->lock);
return;
}
@@ -459,33 +462,34 @@ static int qemudGoDaemon(void) {
static int qemudWritePidFile(const char *pidFile) {
int fd;
FILE *fh;
+ char ebuf[1024];
if (pidFile[0] == '\0')
return 0;
if ((fd = open(pidFile, O_WRONLY|O_CREAT|O_EXCL, 0644)) < 0) {
VIR_ERROR(_("Failed to open pid file '%s' : %s"),
- pidFile, strerror(errno));
+ pidFile, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
if (!(fh = fdopen(fd, "w"))) {
VIR_ERROR(_("Failed to fdopen pid file '%s' : %s"),
- pidFile, strerror(errno));
+ pidFile, virStrerror(errno, ebuf, sizeof ebuf));
close(fd);
return -1;
}
if (fprintf(fh, "%lu\n", (unsigned long)getpid()) < 0) {
VIR_ERROR(_("Failed to write to pid file '%s' : %s"),
- pidFile, strerror(errno));
+ pidFile, virStrerror(errno, ebuf, sizeof ebuf));
close(fd);
return -1;
}
if (fclose(fh) == EOF) {
VIR_ERROR(_("Failed to close pid file '%s' : %s"),
- pidFile, strerror(errno));
+ pidFile, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -498,6 +502,7 @@ static int qemudListenUnix(struct qemud_server *server,
struct sockaddr_un addr;
mode_t oldmask;
gid_t oldgrp;
+ char ebuf[1024];
if (VIR_ALLOC(sock) < 0) {
VIR_ERROR("%s", _("Failed to allocate memory for struct
qemud_socket"));
@@ -511,7 +516,7 @@ static int qemudListenUnix(struct qemud_server *server,
if ((sock->fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) {
VIR_ERROR(_("Failed to create socket: %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
goto cleanup;
}
@@ -533,7 +538,7 @@ static int qemudListenUnix(struct qemud_server *server,
if (bind(sock->fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
VIR_ERROR(_("Failed to bind socket to '%s': %s"),
- path, strerror(errno));
+ path, virStrerror(errno, ebuf, sizeof ebuf));
goto cleanup;
}
umask(oldmask);
@@ -542,7 +547,7 @@ static int qemudListenUnix(struct qemud_server *server,
if (listen(sock->fd, 30) < 0) {
VIR_ERROR(_("Failed to listen for connections on '%s': %s"),
- path, strerror(errno));
+ path, virStrerror(errno, ebuf, sizeof ebuf));
goto cleanup;
}
@@ -587,10 +592,11 @@ remoteMakeSockets (int *fds, int max_fds, int *nfds_r, const char
*node, const c
struct addrinfo *runp = ai;
while (runp && *nfds_r < max_fds) {
+ char ebuf[1024];
fds[*nfds_r] = socket (runp->ai_family, runp->ai_socktype,
runp->ai_protocol);
if (fds[*nfds_r] == -1) {
- VIR_ERROR(_("socket: %s"), strerror (errno));
+ VIR_ERROR(_("socket: %s"), virStrerror (errno, ebuf, sizeof
ebuf));
return -1;
}
@@ -599,14 +605,15 @@ remoteMakeSockets (int *fds, int max_fds, int *nfds_r, const char
*node, const c
if (bind (fds[*nfds_r], runp->ai_addr, runp->ai_addrlen) == -1) {
if (errno != EADDRINUSE) {
- VIR_ERROR(_("bind: %s"), strerror (errno));
+ VIR_ERROR(_("bind: %s"), virStrerror (errno, ebuf, sizeof
ebuf));
return -1;
}
close (fds[*nfds_r]);
}
else {
if (listen (fds[*nfds_r], SOMAXCONN) == -1) {
- VIR_ERROR(_("listen: %s"), strerror (errno));
+ VIR_ERROR(_("listen: %s"),
+ virStrerror (errno, ebuf, sizeof ebuf));
return -1;
}
++*nfds_r;
@@ -638,10 +645,12 @@ remoteListenTCP (struct qemud_server *server,
for (i = 0; i < nfds; ++i) {
struct sockaddr_storage sa;
+ char ebuf[1024];
socklen_t salen = sizeof(sa);
if (VIR_ALLOC(sock) < 0) {
- VIR_ERROR(_("remoteListenTCP: calloc: %s"), strerror (errno));
+ VIR_ERROR(_("remoteListenTCP: calloc: %s"),
+ virStrerror (errno, ebuf, sizeof ebuf));
goto cleanup;
}
@@ -671,7 +680,8 @@ remoteListenTCP (struct qemud_server *server,
goto cleanup;
if (listen (sock->fd, 30) < 0) {
- VIR_ERROR(_("remoteListenTCP: listen: %s"), strerror (errno));
+ VIR_ERROR(_("remoteListenTCP: listen: %s"),
+ virStrerror (errno, ebuf, sizeof ebuf));
goto cleanup;
}
@@ -1145,8 +1155,9 @@ int qemudGetSocketIdentity(int fd, uid_t *uid, pid_t *pid) {
unsigned int cr_len = sizeof (cr);
if (getsockopt (fd, SOL_SOCKET, SO_PEERCRED, &cr, &cr_len) < 0) {
+ char ebuf[1024];
VIR_ERROR(_("Failed to verify client credentials: %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -1169,9 +1180,11 @@ static int qemudDispatchServer(struct qemud_server *server, struct
qemud_socket
int no_slow_start = 1;
if ((fd = accept(sock->fd, (struct sockaddr *)&addr, &addrlen)) < 0) {
+ char ebuf[1024];
if (errno == EAGAIN)
return 0;
- VIR_ERROR(_("Failed to accept connection: %s"), strerror(errno));
+ VIR_ERROR(_("Failed to accept connection: %s"),
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -1476,13 +1489,15 @@ static ssize_t qemudClientReadBuf(struct qemud_client *client,
/*qemudDebug ("qemudClientRead: len = %d", len);*/
if (!client->tlssession) {
+ char ebuf[1024];
ret = read (client->fd, data, len);
if (ret == -1 && (errno == EAGAIN ||
errno == EINTR))
return 0;
if (ret <= 0) {
if (ret != 0)
- VIR_ERROR(_("read: %s"), strerror (errno));
+ VIR_ERROR(_("read: %s"),
+ virStrerror (errno, ebuf, sizeof ebuf));
qemudDispatchClientFailure(client);
return -1;
}
@@ -1700,10 +1715,11 @@ static ssize_t qemudClientWriteBuf(struct qemud_client *client,
}
if (!client->tlssession) {
+ char ebuf[1024];
if ((ret = write(client->fd, data, len)) == -1) {
if (errno == EAGAIN || errno == EINTR)
return 0;
- VIR_ERROR(_("write: %s"), strerror (errno));
+ VIR_ERROR(_("write: %s"), virStrerror (errno, ebuf, sizeof ebuf));
qemudDispatchClientFailure(client);
return -1;
}
@@ -2004,9 +2020,10 @@ static int qemudOneLoop(void) {
/* Check for any signal handling errors and log them. */
errors = sig_errors;
if (errors) {
+ char ebuf[1024];
sig_errors -= errors;
VIR_ERROR(_("Signal handler reported %d errors: last error: %s"),
- errors, strerror (sig_lasterrno));
+ errors, virStrerror (sig_lasterrno, ebuf, sizeof ebuf));
return -1;
}
@@ -2743,8 +2760,10 @@ int main(int argc, char **argv) {
}
if (godaemon) {
+ char ebuf[1024];
if (qemudGoDaemon() < 0) {
- VIR_ERROR(_("Failed to fork as daemon: %s"), strerror(errno));
+ VIR_ERROR(_("Failed to fork as daemon: %s"),
+ virStrerror(errno, ebuf, sizeof ebuf));
goto error1;
}
}
@@ -2765,7 +2784,9 @@ int main(int argc, char **argv) {
qemudSetNonBlock(sigpipe[1]) < 0 ||
qemudSetCloseExec(sigpipe[0]) < 0 ||
qemudSetCloseExec(sigpipe[1]) < 0) {
- VIR_ERROR(_("Failed to create pipe: %s"), strerror(errno));
+ char ebuf[1024];
+ VIR_ERROR(_("Failed to create pipe: %s"),
+ virStrerror(errno, ebuf, sizeof ebuf));
goto error2;
}
sigwrite = sigpipe[1];
--
1.6.1.2.511.gc5d3f
From e5d1e5781fb9f7b4e04bbb51908897b0d732148d Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 2 Feb 2009 12:20:04 +0100
Subject: [PATCH 08/10] don't include raw errno in diagnostics
* src/uml_driver.c (umlStartVMDaemon): Don't print raw errno value.
* qemud/remote.c (remoteDispatchAuthSaslInit): Likewise.
---
qemud/remote.c | 10 +++++-----
src/uml_driver.c | 22 +++++++++++-----------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/qemud/remote.c b/qemud/remote.c
index d064d61..2e0c48e 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -1,7 +1,7 @@
/*
* remote.c: code handling remote requests (from remote_internal.c)
*
- * Copyright (C) 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2007, 2008, 2009 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -2572,8 +2572,8 @@ remoteDispatchAuthSaslInit (struct qemud_server *server,
salen = sizeof(sa);
if (getsockname(client->fd, (struct sockaddr*)&sa, &salen) < 0) {
remoteDispatchFormatError(rerr,
- _("failed to get sock address %d (%s)"),
- errno, strerror(errno));
+ _("failed to get sock address: %s"),
+ strerror(errno));
goto error;
}
if ((localAddr = addrToString(rerr, &sa, salen)) == NULL) {
@@ -2583,8 +2583,8 @@ remoteDispatchAuthSaslInit (struct qemud_server *server,
/* Get remote address in form IPADDR:PORT */
salen = sizeof(sa);
if (getpeername(client->fd, (struct sockaddr*)&sa, &salen) < 0) {
- remoteDispatchFormatError(rerr, _("failed to get peer address %d
(%s)"),
- errno, strerror(errno));
+ remoteDispatchFormatError(rerr, _("failed to get peer address: %s"),
+ strerror(errno));
VIR_FREE(localAddr);
goto error;
}
diff --git a/src/uml_driver.c b/src/uml_driver.c
index 1d12406..0fe2fac 100644
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -1,7 +1,7 @@
/*
* uml_driver.c: core driver methods for managing UML guests
*
- * Copyright (C) 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -791,26 +791,26 @@ static int umlStartVMDaemon(virConnectPtr conn,
tmp = progenv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile: %s\n"),
+ strerror(errno));
if (safewrite(logfd, " ", 1) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile: %s\n"),
+ strerror(errno));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
+ strerror(errno));
if (safewrite(logfd, " ", 1) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
+ strerror(errno));
tmp++;
}
if (safewrite(logfd, "\n", 1) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile %d: %s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
+ strerror(errno));
vm->monitor = -1;
--
1.6.1.2.511.gc5d3f
From ca8d2f5fb341a5624603fb6b99adb6412f66b8a6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 2 Feb 2009 13:34:02 +0100
Subject: [PATCH 09/10] remove remainder of offending strerror uses
* qemud/qemud.c (GET_CONF_STR): Use virStrerror, not strerror.
* qemud/remote.c (remoteDispatchDomainBlockPeek): Likewise.
(remoteDispatchDomainMemoryPeek, remoteDispatchAuthSaslInit): Likewise.
(remoteDispatchAuthPolkit): Likewise.
* src/lxc_container.c (lxcContainerAvailable): Likewise.
* src/network_driver.c (networkStartNetworkDaemon): Likewise.
(networkShutdownNetworkDaemon): Likewise.
* src/qemu_conf.c (qemudExtractVersion, qemudNetworkIfaceConnect):
* src/storage_conf.c (virStoragePoolLoadAllConfigs): Likewise.
* src/storage_driver.c (storagePoolUndefine): Likewise.
* src/uml_driver.c (umlStartup, umlStartVMDaemon): Likewise.
* src/util.c (virFileReadAll): Likewise.
* src/uuid.c (virUUIDGenerate): Likewise.
* src/xen_internal.c (get_cpu_flags): Likewise.
---
qemud/qemud.c | 5 ++++-
qemud/remote.c | 26 ++++++++++++++++++--------
src/lxc_container.c | 4 ++--
src/network_driver.c | 11 +++++++----
src/qemu_conf.c | 8 +++++---
src/storage_conf.c | 5 +++--
src/storage_driver.c | 8 +++++---
src/uml_driver.c | 16 +++++++++-------
src/util.c | 6 ++++--
src/uuid.c | 10 +++++++---
src/xen_internal.c | 3 ++-
11 files changed, 66 insertions(+), 36 deletions(-)
diff --git a/qemud/qemud.c b/qemud/qemud.c
index 48083df..effb336 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -49,6 +49,7 @@
#include <netdb.h>
#include "libvirt_internal.h"
+#include "virterror_internal.h"
#include "qemud.h"
#include "util.h"
@@ -2316,7 +2317,9 @@ checkType (virConfValuePtr p, const char *filename,
goto free_and_fail; \
(var_name) = strdup (p->str); \
if ((var_name) == NULL) { \
- VIR_ERROR(_("remoteReadConfigFile: %s\n"),strerror (errno)); \
+ char ebuf[1024]; \
+ VIR_ERROR(_("remoteReadConfigFile: %s\n"), \
+ virStrerror(errno, ebuf, sizeof ebuf)); \
goto free_and_fail; \
} \
} \
diff --git a/qemud/remote.c b/qemud/remote.c
index 2e0c48e..78dda42 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -41,6 +41,7 @@
#include <string.h>
#include <errno.h>
#include <fnmatch.h>
+#include "virterror_internal.h"
#ifdef HAVE_POLKIT
#include <polkit/polkit.h>
@@ -990,9 +991,11 @@ remoteDispatchDomainBlockPeek (struct qemud_server *server
ATTRIBUTE_UNUSED,
}
ret->buffer.buffer_len = size;
- if (VIR_ALLOC_N(ret->buffer.buffer_val, size) < 0) {
+ if (VIR_ALLOC_N (ret->buffer.buffer_val, size) < 0) {
+ char ebuf[1024];
virDomainFree (dom);
- remoteDispatchFormatError (rerr, "%s", strerror (errno));
+ remoteDispatchFormatError (rerr, "%s",
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -1031,16 +1034,18 @@ remoteDispatchDomainMemoryPeek (struct qemud_server *server
ATTRIBUTE_UNUSED,
flags = args->flags;
if (size > REMOTE_DOMAIN_MEMORY_PEEK_BUFFER_MAX) {
+ virDomainFree (dom);
remoteDispatchFormatError (rerr,
"%s", _("size > maximum buffer
size"));
- virDomainFree (dom);
return -1;
}
ret->buffer.buffer_len = size;
if (VIR_ALLOC_N (ret->buffer.buffer_val, size) < 0) {
- remoteDispatchFormatError (rerr, "%s", strerror (errno));
+ char ebuf[1024];
virDomainFree (dom);
+ remoteDispatchFormatError (rerr, "%s",
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -2571,9 +2576,10 @@ remoteDispatchAuthSaslInit (struct qemud_server *server,
/* Get local address in form IPADDR:PORT */
salen = sizeof(sa);
if (getsockname(client->fd, (struct sockaddr*)&sa, &salen) < 0) {
+ char ebuf[1024];
remoteDispatchFormatError(rerr,
_("failed to get sock address: %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
goto error;
}
if ((localAddr = addrToString(rerr, &sa, salen)) == NULL) {
@@ -2583,8 +2589,9 @@ remoteDispatchAuthSaslInit (struct qemud_server *server,
/* Get remote address in form IPADDR:PORT */
salen = sizeof(sa);
if (getpeername(client->fd, (struct sockaddr*)&sa, &salen) < 0) {
+ char ebuf[1024];
remoteDispatchFormatError(rerr, _("failed to get peer address: %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
VIR_FREE(localAddr);
goto error;
}
@@ -3062,7 +3069,9 @@ remoteDispatchAuthPolkit (struct qemud_server *server,
}
if (!(pkaction = polkit_action_new())) {
- VIR_ERROR(_("Failed to create polkit action %s\n"), strerror(errno));
+ char ebuf[1024];
+ VIR_ERROR(_("Failed to create polkit action %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
polkit_caller_unref(pkcaller);
goto authfail;
}
@@ -3070,9 +3079,10 @@ remoteDispatchAuthPolkit (struct qemud_server *server,
if (!(pkcontext = polkit_context_new()) ||
!polkit_context_init(pkcontext, &pkerr)) {
+ char ebuf[1024];
VIR_ERROR(_("Failed to create polkit context %s\n"),
(pkerr ? polkit_error_get_error_message(pkerr)
- : strerror(errno)));
+ : virStrerror(errno, ebuf, sizeof ebuf)));
if (pkerr)
polkit_error_free(pkerr);
polkit_caller_unref(pkcaller);
diff --git a/src/lxc_container.c b/src/lxc_container.c
index ea52eed..f1c523b 100644
--- a/src/lxc_container.c
+++ b/src/lxc_container.c
@@ -668,8 +668,9 @@ int lxcContainerAvailable(int features)
cpid = clone(lxcContainerDummyChild, childStack, flags, NULL);
VIR_FREE(stack);
if (cpid < 0) {
+ char ebuf[1024];
DEBUG("clone call returned %s, container support is not enabled",
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
} else {
waitpid(cpid, &childStatus, 0);
@@ -677,4 +678,3 @@ int lxcContainerAvailable(int features)
return 0;
}
-
diff --git a/src/network_driver.c b/src/network_driver.c
index dcc6ad9..4138939 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -907,14 +907,16 @@ static int networkStartNetworkDaemon(virConnectPtr conn,
err_delbr1:
if (network->def->ipAddress &&
(err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) {
+ char ebuf[1024];
networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' :
%s\n"),
- network->def->bridge, strerror(err));
+ network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
err_delbr:
if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) {
+ char ebuf[1024];
networkLog(NETWORK_WARN, _("Failed to delete bridge '%s' :
%s\n"),
- network->def->bridge, strerror(err));
+ network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
return -1;
@@ -944,15 +946,16 @@ static int networkShutdownNetworkDaemon(virConnectPtr conn,
networkRemoveIptablesRules(driver, network);
+ char ebuf[1024];
if (network->def->ipAddress &&
(err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) {
networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' :
%s\n"),
- network->def->bridge, strerror(err));
+ network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) {
networkLog(NETWORK_WARN, _("Failed to delete bridge '%s' :
%s\n"),
- network->def->bridge, strerror(err));
+ network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
/* See if its still alive and really really kill it */
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index ef45b12..6f58ee8 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -517,9 +517,10 @@ int qemudExtractVersion(virConnectPtr conn,
return -1;
if (stat(binary, &sb) < 0) {
+ char ebuf[1024];
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Cannot find QEMU binary %s: %s"), binary,
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -580,10 +581,11 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
}
}
+ char ebuf[1024];
if (!driver->brctl && (err = brInit(&driver->brctl))) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("cannot initialize bridge support: %s"),
- strerror(err));
+ virStrerror(err, ebuf, sizeof ebuf));
goto error;
}
@@ -598,7 +600,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to add tap interface '%s' "
"to bridge '%s' : %s"),
- net->ifname, brname, strerror(err));
+ net->ifname, brname, virStrerror(err, ebuf, sizeof
ebuf));
}
goto error;
}
diff --git a/src/storage_conf.c b/src/storage_conf.c
index 7eb89e7..70107a2 100644
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -1,7 +1,7 @@
/*
* storage_conf.c: config handling for storage driver
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -1384,10 +1384,11 @@ virStoragePoolLoadAllConfigs(virConnectPtr conn,
struct dirent *entry;
if (!(dir = opendir(configDir))) {
+ char ebuf[1024];
if (errno == ENOENT)
return 0;
virStorageLog("Failed to open dir '%s': %s",
- configDir, strerror(errno));
+ configDir, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
diff --git a/src/storage_driver.c b/src/storage_driver.c
index a456061..f1320c5 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -1,7 +1,7 @@
/*
* storage_driver.c: core driver for storage APIs
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -567,9 +567,11 @@ storagePoolUndefine(virStoragePoolPtr obj) {
if (virStoragePoolObjDeleteDef(obj->conn, pool) < 0)
goto cleanup;
- if (unlink(pool->autostartLink) < 0 && errno != ENOENT && errno
!= ENOTDIR)
+ if (unlink(pool->autostartLink) < 0 && errno != ENOENT && errno
!= ENOTDIR) {
+ char ebuf[1024];
storageLog("Failed to delete autostart link '%s': %s",
- pool->autostartLink, strerror(errno));
+ pool->autostartLink, virStrerror(errno, ebuf, sizeof ebuf));
+ }
VIR_FREE(pool->configFile);
VIR_FREE(pool->autostartLink);
diff --git a/src/uml_driver.c b/src/uml_driver.c
index 0fe2fac..c5a06a2 100644
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -376,8 +376,9 @@ umlStartup(void) {
}
if (virFileMakePath(uml_driver->monitorDir) < 0) {
+ char ebuf[1024];
umlLog(VIR_LOG_ERROR, _("Failed to create monitor directory %s: %s"),
- uml_driver->monitorDir, strerror(errno));
+ uml_driver->monitorDir, virStrerror(errno, ebuf, sizeof ebuf));
goto error;
}
@@ -726,6 +727,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
int *tapfds = NULL;
int ntapfds = 0;
fd_set keepfd;
+ char ebuf[1024];
FD_ZERO(&keepfd);
@@ -792,25 +794,25 @@ static int umlStartVMDaemon(virConnectPtr conn,
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
if (safewrite(logfd, " ", 1) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
if (safewrite(logfd, " ", 1) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
tmp++;
}
if (safewrite(logfd, "\n", 1) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
vm->monitor = -1;
@@ -825,7 +827,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
/* Cleanup intermediate proces */
if (waitpid(pid, NULL, 0) != pid)
umlLog(VIR_LOG_WARN, _("failed to wait on process: %d: %s\n"),
- pid, strerror(errno));
+ pid, virStrerror(errno, ebuf, sizeof ebuf));
for (i = 0 ; argv[i] ; i++)
VIR_FREE(argv[i]);
diff --git a/src/util.c b/src/util.c
index 96c1b00..01fe37a 100644
--- a/src/util.c
+++ b/src/util.c
@@ -755,17 +755,19 @@ int virFileReadLimFD(int fd_arg, int maxlen, char **buf)
int virFileReadAll(const char *path, int maxlen, char **buf)
{
+ char ebuf[1024];
FILE *fh = fopen(path, "r");
if (fh == NULL) {
virLog("Failed to open file '%s': %s\n",
- path, strerror(errno));
+ path, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
int len = virFileReadLimFP (fh, maxlen, buf);
fclose(fh);
if (len < 0) {
- virLog("Failed to read '%s': %s\n", path, strerror (errno));
+ virLog("Failed to read '%s': %s\n", path,
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
diff --git a/src/uuid.c b/src/uuid.c
index 9d263de..6f7d85f 100644
--- a/src/uuid.c
+++ b/src/uuid.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2007, 2008, 2009 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -36,6 +36,7 @@
#include "c-ctype.h"
#include "internal.h"
#include "util.h"
+#include "virterror_internal.h"
#define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -99,10 +100,13 @@ virUUIDGenerate(unsigned char *uuid)
if (uuid == NULL)
return(-1);
- if ((err = virUUIDGenerateRandomBytes(uuid, VIR_UUID_BUFLEN)))
+ if ((err = virUUIDGenerateRandomBytes(uuid, VIR_UUID_BUFLEN))) {
+ char ebuf[1024];
qemudLog(QEMUD_WARN,
_("Falling back to pseudorandom UUID,"
- " failed to generate random bytes: %s"), strerror(err));
+ " failed to generate random bytes: %s"),
+ virStrerror(err, ebuf, sizeof ebuf));
+ }
return virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN);
}
diff --git a/src/xen_internal.c b/src/xen_internal.c
index 0a01f5e..a866af1 100644
--- a/src/xen_internal.c
+++ b/src/xen_internal.c
@@ -2265,8 +2265,9 @@ get_cpu_flags(virConnectPtr conn, const char **hvm, int *pae, int
*longmode)
if ((fd = open("/dev/cpu/self/cpuid", O_RDONLY)) == -1 ||
pread(fd, ®s, sizeof(regs), 0) != sizeof(regs)) {
+ char ebuf[1024];
virXenError(conn, VIR_ERR_SYSTEM_ERROR,
- "couldn't read CPU flags: %s", strerror(errno));
+ "couldn't read CPU flags: %s", virStrerror(errno, ebuf, sizeof
ebuf));
goto out;
}
--
1.6.1.2.511.gc5d3f
From 214b4d2b01559f85198f16fa34a877407f457c44 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Mon, 2 Feb 2009 13:47:05 +0100
Subject: [PATCH 10/10] syntax-check: enable prohibit_nonreentrant
* .x-sc_prohibit_nonreentrant: Add src/console.c and virterror.c.
* Makefile.cfg (local-checks-to-skip): Don't exempt
sc_prohibit_nonreentrant.
* Makefile.nonreentrant: Don't exempt strerror.
---
.x-sc_prohibit_nonreentrant | 4 +++-
Makefile.nonreentrant | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/.x-sc_prohibit_nonreentrant b/.x-sc_prohibit_nonreentrant
index a90e010..cc323b0 100644
--- a/.x-sc_prohibit_nonreentrant
+++ b/.x-sc_prohibit_nonreentrant
@@ -4,5 +4,7 @@ ChangeLog
^Makefile*
^docs/
^tests/
-^src/virsh.c
+^src/virsh\.c
+^src/console\.c
+^src/virterror\.c
^build-aux/
diff --git a/Makefile.nonreentrant b/Makefile.nonreentrant
index 13fa59d..b567f31 100644
--- a/Makefile.nonreentrant
+++ b/Makefile.nonreentrant
@@ -79,7 +79,7 @@ NON_REENTRANT += setstate
NON_REENTRANT += sgetspent
NON_REENTRANT += srand48
NON_REENTRANT += srandom
-# NON_REENTRANT += strerror
+NON_REENTRANT += strerror
NON_REENTRANT += strtok
NON_REENTRANT += tmpnam
NON_REENTRANT += ttyname
--
1.6.1.2.511.gc5d3f
9:45 a.m.
New subject: [libvirt] [PATCH 4/9] iptables.c: Use virStrerror, not strerror.
On Thu, Feb 05, 2009 at 02:49:50PM +0100, Jim Meyering wrote:
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
> On Thu, Feb 05, 2009 at 02:30:02PM +0100, Jim Meyering wrote:
>> "Daniel P. Berrange" <berrange(a)redhat.com> wrote:
>> > On Wed, Feb 04, 2009 at 01:07:49PM +0100, Jim Meyering wrote:
>> ...
>> >> Here's an incremental diff that also includes additional changes,
e.g.,
>> ^^^^^^^^^^^^^^^^
>
>> > [snip more of the same]
>> >
>> > Have you got the actual patch against current CVS, since this doesn't
>> > appear to be it ?
>>
>> Yep. It was attached at the end, after this:
>>
>> >> Here's the 10-change-set sequence:
>
> This is the section I was quoting - it a seems to be identical to the
> incremental diff.
ACK, this makes more sense now
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:08 a.m.
New subject: [libvirt] [PATCH 5/9] qemud.c: use virStrerror, not strerror
From: Jim Meyering <meyering(a)redhat.com>
---
qemud/qemud.c | 65 +++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/qemud/qemud.c b/qemud/qemud.c
index fa5e17d..48083df 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -231,8 +231,9 @@ remoteCheckCertFile(const char *type, const char *file)
{
struct stat sb;
if (stat(file, &sb) < 0) {
- VIR_ERROR(_("Cannot access %s '%s': %s (%d)"),
- type, file, strerror(errno), errno);
+ char ebuf[1024];
+ VIR_ERROR(_("Cannot access %s '%s': %s"),
+ type, file, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
return 0;
@@ -331,7 +332,9 @@ qemudDispatchSignalEvent(int watch ATTRIBUTE_UNUSED,
virMutexLock(&server->lock);
if (saferead(server->sigread, &siginfo, sizeof(siginfo)) != sizeof(siginfo))
{
- VIR_ERROR(_("Failed to read from signal pipe: %s"), strerror(errno));
+ char ebuf[1024];
+ VIR_ERROR(_("Failed to read from signal pipe: %s"),
+ virStrerror(errno, ebuf, sizeof ebuf));
virMutexUnlock(&server->lock);
return;
}
@@ -459,33 +462,34 @@ static int qemudGoDaemon(void) {
static int qemudWritePidFile(const char *pidFile) {
int fd;
FILE *fh;
+ char ebuf[1024];
if (pidFile[0] == '\0')
return 0;
if ((fd = open(pidFile, O_WRONLY|O_CREAT|O_EXCL, 0644)) < 0) {
VIR_ERROR(_("Failed to open pid file '%s' : %s"),
- pidFile, strerror(errno));
+ pidFile, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
if (!(fh = fdopen(fd, "w"))) {
VIR_ERROR(_("Failed to fdopen pid file '%s' : %s"),
- pidFile, strerror(errno));
+ pidFile, virStrerror(errno, ebuf, sizeof ebuf));
close(fd);
return -1;
}
if (fprintf(fh, "%lu\n", (unsigned long)getpid()) < 0) {
VIR_ERROR(_("Failed to write to pid file '%s' : %s"),
- pidFile, strerror(errno));
+ pidFile, virStrerror(errno, ebuf, sizeof ebuf));
close(fd);
return -1;
}
if (fclose(fh) == EOF) {
VIR_ERROR(_("Failed to close pid file '%s' : %s"),
- pidFile, strerror(errno));
+ pidFile, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -498,6 +502,7 @@ static int qemudListenUnix(struct qemud_server *server,
struct sockaddr_un addr;
mode_t oldmask;
gid_t oldgrp;
+ char ebuf[1024];
if (VIR_ALLOC(sock) < 0) {
VIR_ERROR("%s", _("Failed to allocate memory for struct
qemud_socket"));
@@ -511,7 +516,7 @@ static int qemudListenUnix(struct qemud_server *server,
if ((sock->fd = socket(PF_UNIX, SOCK_STREAM, 0)) < 0) {
VIR_ERROR(_("Failed to create socket: %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
goto cleanup;
}
@@ -533,7 +538,7 @@ static int qemudListenUnix(struct qemud_server *server,
if (bind(sock->fd, (struct sockaddr *)&addr, sizeof(addr)) < 0) {
VIR_ERROR(_("Failed to bind socket to '%s': %s"),
- path, strerror(errno));
+ path, virStrerror(errno, ebuf, sizeof ebuf));
goto cleanup;
}
umask(oldmask);
@@ -542,7 +547,7 @@ static int qemudListenUnix(struct qemud_server *server,
if (listen(sock->fd, 30) < 0) {
VIR_ERROR(_("Failed to listen for connections on '%s': %s"),
- path, strerror(errno));
+ path, virStrerror(errno, ebuf, sizeof ebuf));
goto cleanup;
}
@@ -587,10 +592,11 @@ remoteMakeSockets (int *fds, int max_fds, int *nfds_r, const char
*node, const c
struct addrinfo *runp = ai;
while (runp && *nfds_r < max_fds) {
+ char ebuf[1024];
fds[*nfds_r] = socket (runp->ai_family, runp->ai_socktype,
runp->ai_protocol);
if (fds[*nfds_r] == -1) {
- VIR_ERROR(_("socket: %s"), strerror (errno));
+ VIR_ERROR(_("socket: %s"), virStrerror (errno, ebuf, sizeof
ebuf));
return -1;
}
@@ -599,14 +605,15 @@ remoteMakeSockets (int *fds, int max_fds, int *nfds_r, const char
*node, const c
if (bind (fds[*nfds_r], runp->ai_addr, runp->ai_addrlen) == -1) {
if (errno != EADDRINUSE) {
- VIR_ERROR(_("bind: %s"), strerror (errno));
+ VIR_ERROR(_("bind: %s"), virStrerror (errno, ebuf, sizeof
ebuf));
return -1;
}
close (fds[*nfds_r]);
}
else {
if (listen (fds[*nfds_r], SOMAXCONN) == -1) {
- VIR_ERROR(_("listen: %s"), strerror (errno));
+ VIR_ERROR(_("listen: %s"),
+ virStrerror (errno, ebuf, sizeof ebuf));
return -1;
}
++*nfds_r;
@@ -638,10 +645,12 @@ remoteListenTCP (struct qemud_server *server,
for (i = 0; i < nfds; ++i) {
struct sockaddr_storage sa;
+ char ebuf[1024];
socklen_t salen = sizeof(sa);
if (VIR_ALLOC(sock) < 0) {
- VIR_ERROR(_("remoteListenTCP: calloc: %s"), strerror (errno));
+ VIR_ERROR(_("remoteListenTCP: calloc: %s"),
+ virStrerror (errno, ebuf, sizeof ebuf));
goto cleanup;
}
@@ -671,7 +680,8 @@ remoteListenTCP (struct qemud_server *server,
goto cleanup;
if (listen (sock->fd, 30) < 0) {
- VIR_ERROR(_("remoteListenTCP: listen: %s"), strerror (errno));
+ VIR_ERROR(_("remoteListenTCP: listen: %s"),
+ virStrerror (errno, ebuf, sizeof ebuf));
goto cleanup;
}
@@ -1145,8 +1155,9 @@ int qemudGetSocketIdentity(int fd, uid_t *uid, pid_t *pid) {
unsigned int cr_len = sizeof (cr);
if (getsockopt (fd, SOL_SOCKET, SO_PEERCRED, &cr, &cr_len) < 0) {
+ char ebuf[1024];
VIR_ERROR(_("Failed to verify client credentials: %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -1169,9 +1180,11 @@ static int qemudDispatchServer(struct qemud_server *server, struct
qemud_socket
int no_slow_start = 1;
if ((fd = accept(sock->fd, (struct sockaddr *)&addr, &addrlen)) < 0) {
+ char ebuf[1024];
if (errno == EAGAIN)
return 0;
- VIR_ERROR(_("Failed to accept connection: %s"), strerror(errno));
+ VIR_ERROR(_("Failed to accept connection: %s"),
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -1476,13 +1489,15 @@ static ssize_t qemudClientReadBuf(struct qemud_client *client,
/*qemudDebug ("qemudClientRead: len = %d", len);*/
if (!client->tlssession) {
+ char ebuf[1024];
ret = read (client->fd, data, len);
if (ret == -1 && (errno == EAGAIN ||
errno == EINTR))
return 0;
if (ret <= 0) {
if (ret != 0)
- VIR_ERROR(_("read: %s"), strerror (errno));
+ VIR_ERROR(_("read: %s"),
+ virStrerror (errno, ebuf, sizeof ebuf));
qemudDispatchClientFailure(client);
return -1;
}
@@ -1700,10 +1715,11 @@ static ssize_t qemudClientWriteBuf(struct qemud_client *client,
}
if (!client->tlssession) {
+ char ebuf[1024];
if ((ret = write(client->fd, data, len)) == -1) {
if (errno == EAGAIN || errno == EINTR)
return 0;
- VIR_ERROR(_("write: %s"), strerror (errno));
+ VIR_ERROR(_("write: %s"), virStrerror (errno, ebuf, sizeof ebuf));
qemudDispatchClientFailure(client);
return -1;
}
@@ -2004,9 +2020,10 @@ static int qemudOneLoop(void) {
/* Check for any signal handling errors and log them. */
errors = sig_errors;
if (errors) {
+ char ebuf[1024];
sig_errors -= errors;
VIR_ERROR(_("Signal handler reported %d errors: last error: %s"),
- errors, strerror (sig_lasterrno));
+ errors, virStrerror (sig_lasterrno, ebuf, sizeof ebuf));
return -1;
}
@@ -2743,8 +2760,10 @@ int main(int argc, char **argv) {
}
if (godaemon) {
+ char ebuf[1024];
if (qemudGoDaemon() < 0) {
- VIR_ERROR(_("Failed to fork as daemon: %s"), strerror(errno));
+ VIR_ERROR(_("Failed to fork as daemon: %s"),
+ virStrerror(errno, ebuf, sizeof ebuf));
goto error1;
}
}
@@ -2765,7 +2784,9 @@ int main(int argc, char **argv) {
qemudSetNonBlock(sigpipe[1]) < 0 ||
qemudSetCloseExec(sigpipe[0]) < 0 ||
qemudSetCloseExec(sigpipe[1]) < 0) {
- VIR_ERROR(_("Failed to create pipe: %s"), strerror(errno));
+ char ebuf[1024];
+ VIR_ERROR(_("Failed to create pipe: %s"),
+ virStrerror(errno, ebuf, sizeof ebuf));
goto error2;
}
sigwrite = sigpipe[1];
--
1.6.1.2.418.gd79e6
7:58 a.m.
New subject: [libvirt] [PATCH 5/9] qemud.c: use virStrerror, not strerror
On Mon, Feb 02, 2009 at 06:08:18PM +0100, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
---
qemud/qemud.c | 65 +++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 43 insertions(+), 22 deletions(-)
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:08 a.m.
New subject: [libvirt] [PATCH 6/9] don't include raw errno in diagnostics
From: Jim Meyering <meyering(a)redhat.com>
* src/uml_driver.c (umlStartVMDaemon): Don't print raw errno value.
* qemud/remote.c (remoteDispatchAuthSaslInit): Likewise.
---
qemud/remote.c | 10 +++++-----
src/uml_driver.c | 22 +++++++++++-----------
2 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/qemud/remote.c b/qemud/remote.c
index d064d61..2e0c48e 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -1,7 +1,7 @@
/*
* remote.c: code handling remote requests (from remote_internal.c)
*
- * Copyright (C) 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2007, 2008, 2009 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -2572,8 +2572,8 @@ remoteDispatchAuthSaslInit (struct qemud_server *server,
salen = sizeof(sa);
if (getsockname(client->fd, (struct sockaddr*)&sa, &salen) < 0) {
remoteDispatchFormatError(rerr,
- _("failed to get sock address %d (%s)"),
- errno, strerror(errno));
+ _("failed to get sock address: %s"),
+ strerror(errno));
goto error;
}
if ((localAddr = addrToString(rerr, &sa, salen)) == NULL) {
@@ -2583,8 +2583,8 @@ remoteDispatchAuthSaslInit (struct qemud_server *server,
/* Get remote address in form IPADDR:PORT */
salen = sizeof(sa);
if (getpeername(client->fd, (struct sockaddr*)&sa, &salen) < 0) {
- remoteDispatchFormatError(rerr, _("failed to get peer address %d
(%s)"),
- errno, strerror(errno));
+ remoteDispatchFormatError(rerr, _("failed to get peer address: %s"),
+ strerror(errno));
VIR_FREE(localAddr);
goto error;
}
diff --git a/src/uml_driver.c b/src/uml_driver.c
index 1d12406..0fe2fac 100644
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -1,7 +1,7 @@
/*
* uml_driver.c: core driver methods for managing UML guests
*
- * Copyright (C) 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2006, 2007, 2008, 2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -791,26 +791,26 @@ static int umlStartVMDaemon(virConnectPtr conn,
tmp = progenv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile: %s\n"),
+ strerror(errno));
if (safewrite(logfd, " ", 1) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile: %s\n"),
+ strerror(errno));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
+ strerror(errno));
if (safewrite(logfd, " ", 1) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile %d:
%s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
+ strerror(errno));
tmp++;
}
if (safewrite(logfd, "\n", 1) < 0)
- umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile %d: %s\n"),
- errno, strerror(errno));
+ umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
+ strerror(errno));
vm->monitor = -1;
--
1.6.1.2.418.gd79e6
7:58 a.m.
New subject: [libvirt] [PATCH 6/9] don't include raw errno in diagnostics
On Mon, Feb 02, 2009 at 06:08:19PM +0100, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
* src/uml_driver.c (umlStartVMDaemon): Don't print raw errno value.
* qemud/remote.c (remoteDispatchAuthSaslInit): Likewise.
---
qemud/remote.c | 10 +++++-----
src/uml_driver.c | 22 +++++++++++-----------
2 files changed, 16 insertions(+), 16 deletions(-)
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:08 a.m.
New subject: [libvirt] [PATCH 7/9] remove remainder of offending strerror uses
From: Jim Meyering <meyering(a)redhat.com>
* qemud/qemud.c (GET_CONF_STR): Use virStrerror, not strerror.
* qemud/remote.c (remoteDispatchDomainBlockPeek): Likewise.
(remoteDispatchDomainMemoryPeek, remoteDispatchAuthSaslInit): Likewise.
(remoteDispatchAuthPolkit): Likewise.
* src/lxc_container.c (lxcContainerAvailable): Likewise.
* src/network_driver.c (networkStartNetworkDaemon): Likewise.
(networkShutdownNetworkDaemon): Likewise.
* src/qemu_conf.c (qemudExtractVersion, qemudNetworkIfaceConnect):
* src/storage_conf.c (virStoragePoolLoadAllConfigs): Likewise.
* src/storage_driver.c (storagePoolUndefine): Likewise.
* src/uml_driver.c (umlStartup, umlStartVMDaemon): Likewise.
* src/util.c (virFileReadAll): Likewise.
* src/uuid.c (virUUIDGenerate): Likewise.
* src/xen_internal.c (get_cpu_flags): Likewise.
---
qemud/qemud.c | 5 ++++-
qemud/remote.c | 26 ++++++++++++++++++--------
src/lxc_container.c | 4 ++--
src/network_driver.c | 12 +++++++-----
src/qemu_conf.c | 8 +++++---
src/storage_conf.c | 5 +++--
src/storage_driver.c | 5 +++--
src/uml_driver.c | 16 +++++++++-------
src/util.c | 6 ++++--
src/uuid.c | 7 +++++--
src/xen_internal.c | 5 +++--
11 files changed, 63 insertions(+), 36 deletions(-)
diff --git a/qemud/qemud.c b/qemud/qemud.c
index 48083df..effb336 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -49,6 +49,7 @@
#include <netdb.h>
#include "libvirt_internal.h"
+#include "virterror_internal.h"
#include "qemud.h"
#include "util.h"
@@ -2316,7 +2317,9 @@ checkType (virConfValuePtr p, const char *filename,
goto free_and_fail; \
(var_name) = strdup (p->str); \
if ((var_name) == NULL) { \
- VIR_ERROR(_("remoteReadConfigFile: %s\n"),strerror (errno)); \
+ char ebuf[1024]; \
+ VIR_ERROR(_("remoteReadConfigFile: %s\n"), \
+ virStrerror(errno, ebuf, sizeof ebuf)); \
goto free_and_fail; \
} \
} \
diff --git a/qemud/remote.c b/qemud/remote.c
index 2e0c48e..78dda42 100644
--- a/qemud/remote.c
+++ b/qemud/remote.c
@@ -41,6 +41,7 @@
#include <string.h>
#include <errno.h>
#include <fnmatch.h>
+#include "virterror_internal.h"
#ifdef HAVE_POLKIT
#include <polkit/polkit.h>
@@ -990,9 +991,11 @@ remoteDispatchDomainBlockPeek (struct qemud_server *server
ATTRIBUTE_UNUSED,
}
ret->buffer.buffer_len = size;
- if (VIR_ALLOC_N(ret->buffer.buffer_val, size) < 0) {
+ if (VIR_ALLOC_N (ret->buffer.buffer_val, size) < 0) {
+ char ebuf[1024];
virDomainFree (dom);
- remoteDispatchFormatError (rerr, "%s", strerror (errno));
+ remoteDispatchFormatError (rerr, "%s",
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -1031,16 +1034,18 @@ remoteDispatchDomainMemoryPeek (struct qemud_server *server
ATTRIBUTE_UNUSED,
flags = args->flags;
if (size > REMOTE_DOMAIN_MEMORY_PEEK_BUFFER_MAX) {
+ virDomainFree (dom);
remoteDispatchFormatError (rerr,
"%s", _("size > maximum buffer
size"));
- virDomainFree (dom);
return -1;
}
ret->buffer.buffer_len = size;
if (VIR_ALLOC_N (ret->buffer.buffer_val, size) < 0) {
- remoteDispatchFormatError (rerr, "%s", strerror (errno));
+ char ebuf[1024];
virDomainFree (dom);
+ remoteDispatchFormatError (rerr, "%s",
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -2571,9 +2576,10 @@ remoteDispatchAuthSaslInit (struct qemud_server *server,
/* Get local address in form IPADDR:PORT */
salen = sizeof(sa);
if (getsockname(client->fd, (struct sockaddr*)&sa, &salen) < 0) {
+ char ebuf[1024];
remoteDispatchFormatError(rerr,
_("failed to get sock address: %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
goto error;
}
if ((localAddr = addrToString(rerr, &sa, salen)) == NULL) {
@@ -2583,8 +2589,9 @@ remoteDispatchAuthSaslInit (struct qemud_server *server,
/* Get remote address in form IPADDR:PORT */
salen = sizeof(sa);
if (getpeername(client->fd, (struct sockaddr*)&sa, &salen) < 0) {
+ char ebuf[1024];
remoteDispatchFormatError(rerr, _("failed to get peer address: %s"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
VIR_FREE(localAddr);
goto error;
}
@@ -3062,7 +3069,9 @@ remoteDispatchAuthPolkit (struct qemud_server *server,
}
if (!(pkaction = polkit_action_new())) {
- VIR_ERROR(_("Failed to create polkit action %s\n"), strerror(errno));
+ char ebuf[1024];
+ VIR_ERROR(_("Failed to create polkit action %s\n"),
+ virStrerror(errno, ebuf, sizeof ebuf));
polkit_caller_unref(pkcaller);
goto authfail;
}
@@ -3070,9 +3079,10 @@ remoteDispatchAuthPolkit (struct qemud_server *server,
if (!(pkcontext = polkit_context_new()) ||
!polkit_context_init(pkcontext, &pkerr)) {
+ char ebuf[1024];
VIR_ERROR(_("Failed to create polkit context %s\n"),
(pkerr ? polkit_error_get_error_message(pkerr)
- : strerror(errno)));
+ : virStrerror(errno, ebuf, sizeof ebuf)));
if (pkerr)
polkit_error_free(pkerr);
polkit_caller_unref(pkcaller);
diff --git a/src/lxc_container.c b/src/lxc_container.c
index ea52eed..f1c523b 100644
--- a/src/lxc_container.c
+++ b/src/lxc_container.c
@@ -668,8 +668,9 @@ int lxcContainerAvailable(int features)
cpid = clone(lxcContainerDummyChild, childStack, flags, NULL);
VIR_FREE(stack);
if (cpid < 0) {
+ char ebuf[1024];
DEBUG("clone call returned %s, container support is not enabled",
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
} else {
waitpid(cpid, &childStatus, 0);
@@ -677,4 +678,3 @@ int lxcContainerAvailable(int features)
return 0;
}
-
diff --git a/src/network_driver.c b/src/network_driver.c
index dcc6ad9..8d7340e 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -904,17 +904,18 @@ static int networkStartNetworkDaemon(virConnectPtr conn,
err_delbr2:
networkRemoveIptablesRules(driver, network);
- err_delbr1:
+ err_delbr1:;
+ char ebuf[1024];
if (network->def->ipAddress &&
(err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) {
networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' :
%s\n"),
- network->def->bridge, strerror(err));
+ network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
err_delbr:
if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) {
networkLog(NETWORK_WARN, _("Failed to delete bridge '%s' :
%s\n"),
- network->def->bridge, strerror(err));
+ network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
return -1;
@@ -944,15 +945,16 @@ static int networkShutdownNetworkDaemon(virConnectPtr conn,
networkRemoveIptablesRules(driver, network);
+ char ebuf[1024];
if (network->def->ipAddress &&
(err = brSetInterfaceUp(driver->brctl, network->def->bridge, 0))) {
networkLog(NETWORK_WARN, _("Failed to bring down bridge '%s' :
%s\n"),
- network->def->bridge, strerror(err));
+ network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
if ((err = brDeleteBridge(driver->brctl, network->def->bridge))) {
networkLog(NETWORK_WARN, _("Failed to delete bridge '%s' :
%s\n"),
- network->def->bridge, strerror(err));
+ network->def->bridge, virStrerror(err, ebuf, sizeof ebuf));
}
/* See if its still alive and really really kill it */
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index ef45b12..6f58ee8 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -517,9 +517,10 @@ int qemudExtractVersion(virConnectPtr conn,
return -1;
if (stat(binary, &sb) < 0) {
+ char ebuf[1024];
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Cannot find QEMU binary %s: %s"), binary,
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
@@ -580,10 +581,11 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
}
}
+ char ebuf[1024];
if (!driver->brctl && (err = brInit(&driver->brctl))) {
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("cannot initialize bridge support: %s"),
- strerror(err));
+ virStrerror(err, ebuf, sizeof ebuf));
goto error;
}
@@ -598,7 +600,7 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_("Failed to add tap interface '%s' "
"to bridge '%s' : %s"),
- net->ifname, brname, strerror(err));
+ net->ifname, brname, virStrerror(err, ebuf, sizeof
ebuf));
}
goto error;
}
diff --git a/src/storage_conf.c b/src/storage_conf.c
index 7eb89e7..70107a2 100644
--- a/src/storage_conf.c
+++ b/src/storage_conf.c
@@ -1,7 +1,7 @@
/*
* storage_conf.c: config handling for storage driver
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -1384,10 +1384,11 @@ virStoragePoolLoadAllConfigs(virConnectPtr conn,
struct dirent *entry;
if (!(dir = opendir(configDir))) {
+ char ebuf[1024];
if (errno == ENOENT)
return 0;
virStorageLog("Failed to open dir '%s': %s",
- configDir, strerror(errno));
+ configDir, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
diff --git a/src/storage_driver.c b/src/storage_driver.c
index a456061..d644b63 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -1,7 +1,7 @@
/*
* storage_driver.c: core driver for storage APIs
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -567,9 +567,10 @@ storagePoolUndefine(virStoragePoolPtr obj) {
if (virStoragePoolObjDeleteDef(obj->conn, pool) < 0)
goto cleanup;
+ char ebuf[1024];
if (unlink(pool->autostartLink) < 0 && errno != ENOENT && errno
!= ENOTDIR)
storageLog("Failed to delete autostart link '%s': %s",
- pool->autostartLink, strerror(errno));
+ pool->autostartLink, virStrerror(errno, ebuf, sizeof ebuf));
VIR_FREE(pool->configFile);
VIR_FREE(pool->autostartLink);
diff --git a/src/uml_driver.c b/src/uml_driver.c
index 0fe2fac..48e2d07 100644
--- a/src/uml_driver.c
+++ b/src/uml_driver.c
@@ -376,8 +376,9 @@ umlStartup(void) {
}
if (virFileMakePath(uml_driver->monitorDir) < 0) {
+ char ebuf[1024];
umlLog(VIR_LOG_ERROR, _("Failed to create monitor directory %s: %s"),
- uml_driver->monitorDir, strerror(errno));
+ uml_driver->monitorDir, virStrerror(errno, ebuf, sizeof ebuf));
goto error;
}
@@ -788,29 +789,30 @@ static int umlStartVMDaemon(virConnectPtr conn,
return -1;
}
+ char ebuf[1024];
tmp = progenv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
if (safewrite(logfd, " ", 1) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write envv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
tmp++;
}
tmp = argv;
while (*tmp) {
if (safewrite(logfd, *tmp, strlen(*tmp)) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
if (safewrite(logfd, " ", 1) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
tmp++;
}
if (safewrite(logfd, "\n", 1) < 0)
umlLog(VIR_LOG_WARN, _("Unable to write argv to logfile: %s\n"),
- strerror(errno));
+ virStrerror(errno, ebuf, sizeof ebuf));
vm->monitor = -1;
@@ -825,7 +827,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
/* Cleanup intermediate proces */
if (waitpid(pid, NULL, 0) != pid)
umlLog(VIR_LOG_WARN, _("failed to wait on process: %d: %s\n"),
- pid, strerror(errno));
+ pid, virStrerror(errno, ebuf, sizeof ebuf));
for (i = 0 ; argv[i] ; i++)
VIR_FREE(argv[i]);
diff --git a/src/util.c b/src/util.c
index 6c29872..1034455 100644
--- a/src/util.c
+++ b/src/util.c
@@ -754,17 +754,19 @@ int virFileReadLimFD(int fd_arg, int maxlen, char **buf)
int virFileReadAll(const char *path, int maxlen, char **buf)
{
+ char ebuf[1024];
FILE *fh = fopen(path, "r");
if (fh == NULL) {
virLog("Failed to open file '%s': %s\n",
- path, strerror(errno));
+ path, virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
int len = virFileReadLimFP (fh, maxlen, buf);
fclose(fh);
if (len < 0) {
- virLog("Failed to read '%s': %s\n", path, strerror (errno));
+ virLog("Failed to read '%s': %s\n", path,
+ virStrerror(errno, ebuf, sizeof ebuf));
return -1;
}
diff --git a/src/uuid.c b/src/uuid.c
index 9d263de..9e9416f 100644
--- a/src/uuid.c
+++ b/src/uuid.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2007, 2008, 2009 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -36,6 +36,7 @@
#include "c-ctype.h"
#include "internal.h"
#include "util.h"
+#include "virterror_internal.h"
#define qemudLog(level, msg...) fprintf(stderr, msg)
@@ -99,10 +100,12 @@ virUUIDGenerate(unsigned char *uuid)
if (uuid == NULL)
return(-1);
+ char ebuf[1024];
if ((err = virUUIDGenerateRandomBytes(uuid, VIR_UUID_BUFLEN)))
qemudLog(QEMUD_WARN,
_("Falling back to pseudorandom UUID,"
- " failed to generate random bytes: %s"), strerror(err));
+ " failed to generate random bytes: %s"),
+ virStrerror(err, ebuf, sizeof ebuf));
return virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN);
}
diff --git a/src/xen_internal.c b/src/xen_internal.c
index 9a7272f..4c36b1b 100644
--- a/src/xen_internal.c
+++ b/src/xen_internal.c
@@ -1,7 +1,7 @@
/*
* xen_internal.c: direct access to Xen hypervisor level
*
- * Copyright (C) 2005, 2006, 2007, 2008 Red Hat, Inc.
+ * Copyright (C) 2005, 2006, 2007, 2008, 2009 Red Hat, Inc.
*
* See COPYING.LIB for the License of this software
*
@@ -2265,8 +2265,9 @@ get_cpu_flags(virConnectPtr conn, const char **hvm, int *pae, int
*longmode)
if ((fd = open("/dev/cpu/self/cpuid", O_RDONLY)) == -1 ||
pread(fd, ®s, sizeof(regs), 0) != sizeof(regs)) {
+ char ebuf[1024];
virXenError(conn, VIR_ERR_SYSTEM_ERROR,
- "couldn't read CPU flags: %s", strerror(errno));
+ "couldn't read CPU flags: %s", virStrerror(errno, ebuf, sizeof
ebuf));
goto out;
}
--
1.6.1.2.418.gd79e6
8:01 a.m.
New subject: [libvirt] [PATCH 7/9] remove remainder of offending strerror uses
On Mon, Feb 02, 2009 at 06:08:20PM +0100, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
* qemud/qemud.c (GET_CONF_STR): Use virStrerror, not strerror.
* qemud/remote.c (remoteDispatchDomainBlockPeek): Likewise.
(remoteDispatchDomainMemoryPeek, remoteDispatchAuthSaslInit): Likewise.
(remoteDispatchAuthPolkit): Likewise.
* src/lxc_container.c (lxcContainerAvailable): Likewise.
* src/network_driver.c (networkStartNetworkDaemon): Likewise.
(networkShutdownNetworkDaemon): Likewise.
* src/qemu_conf.c (qemudExtractVersion, qemudNetworkIfaceConnect):
* src/storage_conf.c (virStoragePoolLoadAllConfigs): Likewise.
* src/storage_driver.c (storagePoolUndefine): Likewise.
* src/uml_driver.c (umlStartup, umlStartVMDaemon): Likewise.
* src/util.c (virFileReadAll): Likewise.
* src/uuid.c (virUUIDGenerate): Likewise.
* src/xen_internal.c (get_cpu_flags): Likewise.
diff --git a/src/network_driver.c b/src/network_driver.c
index dcc6ad9..8d7340e 100644
--- a/src/network_driver.c
+++ b/src/network_driver.c
@@ -904,17 +904,18 @@ static int networkStartNetworkDaemon(virConnectPtr conn,
err_delbr2:
networkRemoveIptablesRules(driver, network);
- err_delbr1:
+ err_delbr1:;
+ char ebuf[1024];
A stray ';' crept in there.
diff --git a/src/storage_driver.c b/src/storage_driver.c
index a456061..d644b63 100644
--- a/src/storage_driver.c
+++ b/src/storage_driver.c
@@ -1,7 +1,7 @@
/*
* storage_driver.c: core driver for storage APIs
*
- * Copyright (C) 2006-2008 Red Hat, Inc.
+ * Copyright (C) 2006-2009 Red Hat, Inc.
* Copyright (C) 2006-2008 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -567,9 +567,10 @@ storagePoolUndefine(virStoragePoolPtr obj) {
if (virStoragePoolObjDeleteDef(obj->conn, pool) < 0)
goto cleanup;
+ char ebuf[1024];
if (unlink(pool->autostartLink) < 0 && errno != ENOENT &&
errno != ENOTDIR)
storageLog("Failed to delete autostart link '%s': %s",
- pool->autostartLink, strerror(errno));
+ pool->autostartLink, virStrerror(errno, ebuf, sizeof ebuf));
The variable decl should be inside the if () {} clause there.
@@ -788,29 +789,30 @@ static int umlStartVMDaemon(virConnectPtr
conn,
return -1;
}
+ char ebuf[1024];
tmp = progenv;
while (*tmp) {
Here, the var decl should be inside the while() loop too
@@ -99,10 +100,12 @@ virUUIDGenerate(unsigned char *uuid)
if (uuid == NULL)
return(-1);
+ char ebuf[1024];
if ((err = virUUIDGenerateRandomBytes(uuid, VIR_UUID_BUFLEN)))
qemudLog(QEMUD_WARN,
_("Falling back to pseudorandom UUID,"
- " failed to generate random bytes: %s"), strerror(err));
+ " failed to generate random bytes: %s"),
+ virStrerror(err, ebuf, sizeof ebuf));
return virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN);
AGain the ebuf needs to move inside the block
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
9:06 a.m.
New subject: [libvirt] [PATCH 7/9] remove remainder of offending strerror uses
"Daniel P. Berrange" <berrange(a)redhat.com> wrote:
On Mon, Feb 02, 2009 at 06:08:20PM +0100, Jim Meyering wrote:
> From: Jim Meyering <meyering(a)redhat.com>
Thanks for the review.
> - err_delbr1:
> + err_delbr1:;
> + char ebuf[1024];
A stray ';' crept in there.
Actually, it's required by C.
The ";" adds an empty statement before the new declaration,
to serve as a target for the label.
> diff --git a/src/storage_driver.c b/src/storage_driver.c
> index a456061..d644b63 100644
> --- a/src/storage_driver.c
> +++ b/src/storage_driver.c
> @@ -1,7 +1,7 @@
> /*
> * storage_driver.c: core driver for storage APIs
> *
> - * Copyright (C) 2006-2008 Red Hat, Inc.
> + * Copyright (C) 2006-2009 Red Hat, Inc.
> * Copyright (C) 2006-2008 Daniel P. Berrange
> *
> * This library is free software; you can redistribute it and/or
> @@ -567,9 +567,10 @@ storagePoolUndefine(virStoragePoolPtr obj) {
> if (virStoragePoolObjDeleteDef(obj->conn, pool) < 0)
> goto cleanup;
>
> + char ebuf[1024];
> if (unlink(pool->autostartLink) < 0 && errno != ENOENT &&
errno != ENOTDIR)
> storageLog("Failed to delete autostart link '%s': %s",
> - pool->autostartLink, strerror(errno));
> + pool->autostartLink, virStrerror(errno, ebuf, sizeof ebuf));
The variable decl should be inside the if () {} clause there.
Oops. No idea why I did that.
Fixed.
> @@ -788,29 +789,30 @@ static int umlStartVMDaemon(virConnectPtr
conn,
> return -1;
> }
>
> + char ebuf[1024];
> tmp = progenv;
> while (*tmp) {
Here, the var decl should be inside the while() loop too
Well, if I move the decl into that first while loop, then I'd have
to add 3 more, one into the next while loop, and then two more,
each into currently-single-stmt if-blocks. Seems slightly better
to add just that one decl than to add 4.
> @@ -99,10 +100,12 @@ virUUIDGenerate(unsigned char *uuid)
> if (uuid == NULL)
> return(-1);
>
> + char ebuf[1024];
> if ((err = virUUIDGenerateRandomBytes(uuid, VIR_UUID_BUFLEN)))
> qemudLog(QEMUD_WARN,
> _("Falling back to pseudorandom UUID,"
> - " failed to generate random bytes: %s"),
strerror(err));
> + " failed to generate random bytes: %s"),
> + virStrerror(err, ebuf, sizeof ebuf));
>
> return virUUIDGeneratePseudoRandomBytes(uuid, VIR_UUID_BUFLEN);
AGain the ebuf needs to move inside the block
Good catch. Done.
9:17 a.m.
New subject: [libvirt] [PATCH 7/9] remove remainder of offending strerror uses
On Tue, Feb 03, 2009 at 04:06:18PM +0100, Jim Meyering wrote:
>> - err_delbr1:
>> + err_delbr1:;
>> + char ebuf[1024];
>
> A stray ';' crept in there.
Actually, it's required by C.
The ";" adds an empty statement before the new declaration,
to serve as a target for the label.
That's pretty horrible :-( I'd prefer it to just move the
ebuf inside the following if() {} block where its used, or
to the other var decls at the start of the function.
>> @@ -788,29 +789,30 @@ static int
umlStartVMDaemon(virConnectPtr conn,
>> return -1;
>> }
>>
>> + char ebuf[1024];
>> tmp = progenv;
>> while (*tmp) {
>
> Here, the var decl should be inside the while() loop too
Well, if I move the decl into that first while loop, then I'd have
to add 3 more, one into the next while loop, and then two more,
each into currently-single-stmt if-blocks. Seems slightly better
to add just that one decl than to add 4.
If its used in lots of places in the function, then its clearer to
declare at the start of the function.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:08 a.m.
New subject: [libvirt] [PATCH 8/9] syntax-check: enable prohibit_nonreentrant
From: Jim Meyering <meyering(a)redhat.com>
* .x-sc_prohibit_nonreentrant: Add src/console.c and virterror.c.
* Makefile.cfg (local-checks-to-skip): Don't exempt
sc_prohibit_nonreentrant.
---
.x-sc_prohibit_nonreentrant | 4 +++-
Makefile.cfg | 1 -
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/.x-sc_prohibit_nonreentrant b/.x-sc_prohibit_nonreentrant
index a90e010..cc323b0 100644
--- a/.x-sc_prohibit_nonreentrant
+++ b/.x-sc_prohibit_nonreentrant
@@ -4,5 +4,7 @@ ChangeLog
^Makefile*
^docs/
^tests/
-^src/virsh.c
+^src/virsh\.c
+^src/console\.c
+^src/virterror\.c
^build-aux/
diff --git a/Makefile.cfg b/Makefile.cfg
index 44d3898..4506593 100644
--- a/Makefile.cfg
+++ b/Makefile.cfg
@@ -44,7 +44,6 @@ local-checks-to-skip = \
sc_prohibit_S_IS_definition \
sc_prohibit_atoi_atof \
sc_prohibit_jm_in_m4 \
- sc_prohibit_nonreentrant \
sc_prohibit_quote_without_use \
sc_prohibit_quotearg_without_use \
sc_prohibit_stat_st_blocks \
--
1.6.1.2.418.gd79e6
8:02 a.m.
New subject: [libvirt] [PATCH 8/9] syntax-check: enable prohibit_nonreentrant
On Mon, Feb 02, 2009 at 06:08:21PM +0100, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
* .x-sc_prohibit_nonreentrant: Add src/console.c and virterror.c.
* Makefile.cfg (local-checks-to-skip): Don't exempt
sc_prohibit_nonreentrant.
---
.x-sc_prohibit_nonreentrant | 4 +++-
Makefile.cfg | 1 -
2 files changed, 3 insertions(+), 2 deletions(-)
ACK.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
11:08 a.m.
New subject: [libvirt] [PATCH 9/9] syntax-check: avoid spurious false-positive
From: Jim Meyering <meyering(a)redhat.com>
* src/iptables.c (notifyRulesUpdated): Use %s rather than
string-concatenation that made sc_unmarked_diagnostics report
a false-positive.
---
src/iptables.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/iptables.c b/src/iptables.c
index 81c976b..68f1941 100644
--- a/src/iptables.c
+++ b/src/iptables.c
@@ -101,8 +101,8 @@ notifyRulesUpdated(const char *table,
char ebuf[1024];
if (virRun(NULL, argv, NULL) < 0)
- qemudLog(QEMUD_WARN, _("Failed to run '" LOKKIT_PATH "
%s': %s"),
- arg, virStrerror(errno, ebuf, sizeof ebuf));
+ qemudLog(QEMUD_WARN, _("Failed to run '%s %s': %s"),
+ LOKKIT_PATH, arg, virStrerror(errno, ebuf, sizeof ebuf));
}
static int
--
1.6.1.2.418.gd79e6
8:02 a.m.
New subject: [libvirt] [PATCH 9/9] syntax-check: avoid spurious false-positive
On Mon, Feb 02, 2009 at 06:08:22PM +0100, Jim Meyering wrote:
From: Jim Meyering <meyering(a)redhat.com>
* src/iptables.c (notifyRulesUpdated): Use %s rather than
string-concatenation that made sc_unmarked_diagnostics report
a false-positive.
ACK
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
5769
days inactive
5772
days old
32 comments
2 participants
participants (2)
-
Daniel P. Berrange -
Jim Meyering