On 08/28/2011 08:52 PM, xuhj@xxxxxxxxxxxxxxxxxx wrote:
From: Xu He Jie<xuhj@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Xu He Jie<xuhj@xxxxxxxxxxxxxxxxxx>
Usually this line goes on the bottom, not the top, of a commit message.
When libvirtd is running at non-root user, it won't create
${HOME}/.libvirt.
It will show error message:
17:44:16.838: 7035: error : virPidFileAcquirePath:322 : Failed to
open pid file
+ run_dir = strdup(LOCALSTATEDIR "/run/libvirt");
+ if (!run_dir) {
+ VIR_ERROR(_("Can't allocate memory"));
virReportOOMError(), not a hand-rolled error reporting.
+ goto cleanup;
+ }
+ }
+ else {
Style: '} else {' with no newline.
+ char *user_dir = NULL;
+
+ if (!(user_dir = virGetUserDirectory(geteuid()))) {
+ VIR_ERROR(_("Can't determine user directory"));
+ goto cleanup;
+ }
+
+ if (virAsprintf(&run_dir, "%s/.libvirt/", user_dir)< 0) {
+ VIR_ERROR(_("Can't allocate memory"));
We can consolidate OOM checking for both branches by sinking it below
the if.
+ VIR_FREE(user_dir);
+ goto cleanup;
}
- umask(old_umask);
Hmm, on second thought, virFileMakePath is documented as explicitly
using 0777 - umask, but we really do want the new directory to be 0755
if we create it. I think I'll keep the umask modifications intact (if
someone can convince me otherwise, especially if we have a use case
for 0775 instead of 0777 or 0755, we can change that in a later patch).
+ if (run_dir)
+ VIR_FREE(run_dir);
Fails 'make syntax-check', due to a useless if before free.
ACK with these changes squashed in, so I pushed. I also added you to
AUTHORS; let me know if I need to update anything there.
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index e0004c7..7734fff 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1250,6 +1250,7 @@ int main(int argc, char **argv) {
bool implicit_conf = false;
bool use_polkit_dbus;
char *run_dir = NULL;
+ mode_t old_umask;
struct option opts[] = {
{ "verbose", no_argument, &verbose, 1},
@@ -1405,28 +1406,22 @@ int main(int argc, char **argv) {
/* Ensure the rundir exists (on tmpfs on some systems) */
if (privileged) {
run_dir = strdup(LOCALSTATEDIR "/run/libvirt");
- if (!run_dir) {
- VIR_ERROR(_("Can't allocate memory"));
- goto cleanup;
- }
- }
- else {
- char *user_dir = NULL;
+ } else {
+ char *user_dir = virGetUserDirectory(geteuid());
- if (!(user_dir = virGetUserDirectory(geteuid()))) {
+ if (!user_dir) {
VIR_ERROR(_("Can't determine user directory"));
goto cleanup;
}
-
- if (virAsprintf(&run_dir, "%s/.libvirt/", user_dir) < 0) {
- VIR_ERROR(_("Can't allocate memory"));
- VIR_FREE(user_dir);
- goto cleanup;
- }
-
+ ignore_value(virAsprintf(&run_dir, "%s/.libvirt/", user_dir));
VIR_FREE(user_dir);
}
+ if (!run_dir) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ old_umask = umask(022);
if (virFileMakePath(run_dir) < 0) {
char ebuf[1024];
VIR_ERROR(_("unable to create rundir %s: %s"), run_dir,
@@ -1434,6 +1429,7 @@ int main(int argc, char **argv) {
ret = VIR_DAEMON_ERR_RUNDIR;
goto cleanup;
}
+ umask(old_umask);
/* Try to claim the pidfile, exiting if we can't */
if ((pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) {
@@ -1586,8 +1582,7 @@ cleanup:
VIR_FREE(sock_file_ro);
VIR_FREE(pid_file);
VIR_FREE(remote_config_file);
- if (run_dir)
- VIR_FREE(run_dir);
+ VIR_FREE(run_dir);
daemonConfigFree(config);
virLogShutdown();