On 10 October 2015 at 01:07, Andy Zhou <az...@nicira.com> wrote:
> vlog log file can be created when parsing --log-file option, before
switch user, in case the --user option is also specified. This
>
this does not read fluently. How about:
s/switch user/switching user?
does not directly causing errors for the running daemons, but leaves
>
s/causing/cause
> the log files on disk as owned by root. It can be confusing at best.
>
> This patch fixes the log file ownership setting to match with the
> daemon that writes to it.
> Signed-off-by: Andy Zhou <az...@nicira.com>
> ---
> include/openvswitch/vlog.h | 1 +
> lib/daemon-unix.c | 7 +++++++
> lib/vlog.c | 14 ++++++++++++++
> 3 files changed, 22 insertions(+)
>
> diff --git a/include/openvswitch/vlog.h b/include/openvswitch/vlog.h
> index f6bb3ab..139dfb9 100644
> --- a/include/openvswitch/vlog.h
> +++ b/include/openvswitch/vlog.h
> @@ -143,6 +143,7 @@ void vlog_set_verbosity(const char *arg);
> void vlog_set_pattern(enum vlog_destination, const char *pattern);
> int vlog_set_log_file(const char *file_name);
> int vlog_reopen_log_file(void);
> +int vlog_change_owner(uid_t, gid_t);
>
> /* Configure method how vlog should send messages to syslog server. */
> void vlog_set_syslog_method(const char *method);
> diff --git a/lib/daemon-unix.c b/lib/daemon-unix.c
> index cafa397..e31dbc4 100644
> --- a/lib/daemon-unix.c
> +++ b/lib/daemon-unix.c
> @@ -856,6 +856,13 @@ daemon_become_new_user__(bool access_datapath)
> return;
> }
>
> + /* If vlog file has been created, change its owner to the non-root
> user
> + * as specifed by the --user option. */
> + if (vlog_change_owner(uid, gid)) {
> + VLOG_FATAL("%s: fail to change owner of the log file from root "
>
s/fail/failed
> + "to user %s", pidfile, user);
>
If log file doesn't yet exist on filesystem, wouldn't you call here
VLOG_FATAL? Based on "man 2 chown" it seems that it can return ENOENT if
file does not exist.
> + }
> +
> if (LINUX) {
> if (LIBCAPNG) {
> daemon_become_new_user_linux(access_datapath);
> diff --git a/lib/vlog.c b/lib/vlog.c
> index da31e6f..56b8db8 100644
> --- a/lib/vlog.c
> +++ b/lib/vlog.c
> @@ -430,6 +430,20 @@ vlog_reopen_log_file(void)
> }
> }
>
> +/* In case a log file exists, change its owner to new 'user' and 'group'.
> + *
> + * This is useful for handling cases where the --log-file option is
> + * specified ahead of the --user option. */
>
I think we typically define what is return value in function comments (see
CodingStyle.md).
+int
> +vlog_change_owner(uid_t user, gid_t group)
> +{
> + ovs_mutex_lock(&log_file_mutex);
> + int error = log_file_name ? chown(log_file_name, user, group) : 0;
>
+ ovs_mutex_unlock(&log_file_mutex);
> +
> + return error;
> +}
> +
> /* Set debugging levels. Returns null if successful, otherwise an error
> * message that the caller must free(). */
> char *
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
