Merge lp:~afrantzis/lightdm/logrotate into lp:lightdm

[Alexandros Frantzis]

Use logrotate to handle files in the default log directory

Use logrotate to handle files in the default log directory

This MP introduces support for log rotation using the logrotate tool for file in the default log directory (/var/log/lightdm). To support this scenario, existing log files in the default log directory are not moved to *.old when starting.

[Robert Ancell (robert-ancell)]

I don't like the logic in is_log_file_managed_by_logrotate() - as well as having the log directory hard-coded this is inferring behaviour about how Ubuntu has configured LightDM.

Instead, I think open_log_file() should have a flag to say if the existing file should be backed up. For the system logs (i.e. in /var/log/lightdm, the main log, the X logs, the greeter logs etc) don't do the backup. For the other logs (session log) - do the backup.

Ideally we'd actually have a configuration item to disable log backups but it's not essential and can be added later if necessary.

Also please change the new logging code to match existing style: open_log_file() -> log_file_open()

Thanks!

[Robert Ancell (robert-ancell)]

review: Needs Fixing

[lp:~afrantzis/lightdm/logrotate]

2218. By Robert Ancell on 2015-10-16

Allow guest sessions to write in /{,var/}run/screen folder, so they can launch screen terminals (needed for e.g. Epoptes to open remote clients' terminals).

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:2209
http://jenkins.qa.ubuntu.com/job/lightdm-ci/313/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/lightdm-wily-amd64-ci/3/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/lightdm-wily-armhf-ci/3/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/lightdm-ci/313/rebuild

review: Needs Fixing (continuous-integration)

[Alexandros Frantzis (afrantzis)]

> Instead, I think open_log_file() should have a flag to say if the existing
> file should be backed up. For the system logs (i.e. in /var/log/lightdm, the
> main log, the X logs, the greeter logs etc) don't do the backup. For the other
> logs (session log) - do the backup.

OK. Let me know if the following plan sounds good:

1. process_set_log_file() and session_set_log_file() will get a flag about whether to perform a backup or not, or alternatively we can add *_set_log_file_backup(..., gboolean backup)
2. Session will perform a backup by default (since their default log file is .xsession-errors)
3. At all invocations of *_set_log_file() in the current code the flag will be set to FALSE (don't backup), since all of them are considered system logs (in the current code session logs use the default log value of Session)

> Ideally we'd actually have a configuration item to disable log backups but
> it's not essential and can be added later if necessary.
>
> Also please change the new logging code to match existing style:
> open_log_file() -> log_file_open()

Ack.

[Robert Ancell (robert-ancell)]

Sounds good!

[lp:~afrantzis/lightdm/logrotate]

2219. By Launchpad Translations on behalf of lightdm-team on 2015-10-18

Launchpad automatic translations update.

2220. By Robert Ancell on 2015-10-19

Set example multi-seat configuration to a valid seat name

2221. By Robert Ancell on 2015-10-19

Update guest-session AppArmor profile to be suitable for openSUSE

2222. By Alexandros Frantzis on 2015-10-20

Use logrotate to handle files in the default log directory

Unmerged revisions

=== modified file 'debian/changelog'
--- debian/changelog	2015-10-12 16:56:46 +0000
+++ debian/changelog	2015-10-20 10:30:53 +0000
@@ -1,12 +1,16 @@
 lightdm (1.17.0-0ubuntu1) UNRELEASED; urgency=medium
 
   * New upstream release:
-    - ...
+    - Don't backup system log files to *.old when starting. Handling of these
+      log files is now left to the system (e.g., through logrotate).
   * Build with multi-arch
   * debian/patches/xorg-1.17.patch:
     - Fix xserver-allow-tcp=true option not working with X.org 1.17
   * data/apparmor/abstractions/lightdm_chromium-browser: cgroups support for
     guest sessions. (LP: #1504049, LP: #1464958)
+  * debian/lightdm.logrotate:
+    - Use logrotate to handle log files placed in the default system log
+      directory (/var/log/lightdm).
 
  -- Robert Ancell <[e-mail]>  Mon, 12 Oct 2015 14:57:47 +0100
 

=== added file 'debian/lightdm.logrotate'
--- debian/lightdm.logrotate	1970-01-01 00:00:00 +0000
+++ debian/lightdm.logrotate	2015-10-20 10:30:53 +0000
@@ -0,0 +1,9 @@
+/var/log/lightdm/*.log {
+    daily
+    missingok
+    rotate 7
+    compress
+    notifempty
+    maxsize 10M
+    copytruncate
+}

=== modified file 'src/Makefile.am'
--- src/Makefile.am	2015-08-21 03:20:27 +0000
+++ src/Makefile.am	2015-10-20 10:30:53 +0000
@@ -19,6 +19,9 @@
 	logger.h \
 	login1.c \
 	login1.h \
+	log-file.c \
+	log-file.h \
+	log-mode.h \
 	mir-server.c \
 	mir-server.h \
 	plymouth.c \

=== modified file 'src/lightdm.c'
--- src/lightdm.c	2015-10-16 10:20:03 +0000
+++ src/lightdm.c	2015-10-20 10:30:53 +0000
@@ -33,6 +33,7 @@
 #include "shared-data-manager.h"
 #include "user-list.h"
 #include "login1.h"
+#include "log-file.h"
 
 static gchar *config_path = NULL;
 static GMainLoop *loop = NULL;
@@ -124,7 +125,7 @@
 static void
 log_init (void)
 {
-    gchar *log_dir, *path, *old_path;
+    gchar *log_dir, *path;
 
     log_timer = g_timer_new ();
 
@@ -133,13 +134,7 @@
     path = g_build_filename (log_dir, "lightdm.log", NULL);
     g_free (log_dir);
 
-    /* Move old file out of the way */
-    old_path = g_strdup_printf ("%s.old", path);
-    rename (path, old_path);
-    g_free (old_path);
-
-    /* Create new file and log to it */
-    log_fd = open (path, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+    log_fd = log_file_open (path, LOG_MODE_APPEND);
     fcntl (log_fd, F_SETFD, FD_CLOEXEC);
     g_log_set_default_handler (log_cb, NULL);
 

=== added file 'src/log-file.c'
--- src/log-file.c	1970-01-01 00:00:00 +0000
+++ src/log-file.c	2015-10-20 10:30:53 +0000
@@ -0,0 +1,53 @@
+/*
+ * Copyright (C) 2015 Alexandros Frantzis
+ * Author: Alexandros Frantzis <[e-mail]>
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation, either version 3 of the License, or (at your option) any later
+ * version. See http://www.gnu.org/copyleft/gpl.html the full text of the
+ * license.
+ */
+
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+
+#include "log-file.h"
+
+int
+log_file_open (const gchar *log_filename, LogMode log_mode)
+{
+    int open_flags = O_WRONLY | O_CREAT;
+    int log_fd;
+
+    if (log_mode == LOG_MODE_BACKUP_AND_TRUNCATE)
+    {
+        /* Move old file out of the way */
+        gchar *old_filename;
+
+        old_filename = g_strdup_printf ("%s.old", log_filename);
+        rename (log_filename, old_filename);
+        g_free (old_filename);
+
+        open_flags |= O_TRUNC;
+    }
+    else if (log_mode == LOG_MODE_APPEND)
+    {
+        /* Keep appending to it */
+        open_flags |= O_APPEND;
+    }
+    else
+    {
+        g_warning ("Failed to open log file %s: invalid log mode %d specified",
+                   log_filename, log_mode);
+        return -1;
+    }
+
+    /* Open file and log to it */
+    log_fd = open (log_filename, open_flags, 0600);
+    if (log_fd < 0)
+        g_warning ("Failed to open log file %s: %s", log_filename, g_strerror (errno));
+
+    return log_fd;
+}

=== added file 'src/log-file.h'
--- src/log-file.h	1970-01-01 00:00:00 +0000
+++ src/log-file.h	2015-10-20 10:30:53 +0000
@@ -0,0 +1,21 @@
+/*
+ * Copyright (C) 2015 Alexandros Frantzis
+ * Author: Alexandros Frantzis <[e-mail]>
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation, either version 3 of the License, or (at your option) any later
+ * version. See http://www.gnu.org/copyleft/gpl.html the full text of the
+ * license.
+ */
+
+#ifndef LOG_FILE_H_
+#define LOG_FILE_H_
+
+#include <glib.h>
+
+#include "log-mode.h"
+
+int log_file_open (const gchar *log_filename, LogMode log_mode);
+
+#endif /* !LOG_FILE_H */

=== added file 'src/log-mode.h'
--- src/log-mode.h	1970-01-01 00:00:00 +0000
+++ src/log-mode.h	2015-10-20 10:30:53 +0000
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2015 Alexandros Frantzis
+ * Author: Alexandros Frantzis <[e-mail]>
+ *
+ * This program is free software: you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License as published by the Free Software
+ * Foundation, either version 3 of the License, or (at your option) any later
+ * version. See http://www.gnu.org/copyleft/gpl.html the full text of the
+ * license.
+ */
+
+#ifndef LOG_MODE_H_
+#define LOG_MODE_H_
+
+typedef enum
+{
+    LOG_MODE_INVALID = -1,
+    LOG_MODE_BACKUP_AND_TRUNCATE,
+    LOG_MODE_APPEND
+} LogMode;
+
+#endif /* !LOD_MODE_H_ */

=== modified file 'src/process.c'
--- src/process.c	2015-10-16 10:04:18 +0000
+++ src/process.c	2015-10-20 10:30:53 +0000
@@ -17,9 +17,9 @@
 #include <fcntl.h>
 #include <signal.h>
 #include <grp.h>
-#include <glib/gstdio.h>
 #include <config.h>
 
+#include "log-file.h"
 #include "process.h"
 
 enum {
@@ -39,6 +39,7 @@
     /* File to log to */
     gchar *log_file;
     gboolean log_stdout;
+    LogMode log_mode;
 
     /* Command to run */
     gchar *command;
@@ -90,16 +91,18 @@
     Process *process = g_object_new (PROCESS_TYPE, NULL);
     process->priv->run_func = run_func;
     process->priv->run_func_data = run_func_data;
+    process->priv->log_mode = LOG_MODE_INVALID;
     return process;
 }
 
 void
-process_set_log_file (Process *process, const gchar *path, gboolean log_stdout)
+process_set_log_file (Process *process, const gchar *path, gboolean log_stdout, LogMode log_mode)
 {
     g_return_if_fail (process != NULL);
     g_free (process->priv->log_file);
     process->priv->log_file = g_strdup (path);
     process->priv->log_stdout = log_stdout;
+    process->priv->log_mode = log_mode;
 }
 
 void
@@ -193,19 +196,7 @@
     }
 
     if (process->priv->log_file)
-    {
-        gchar *old_filename;
-
-        /* Move old file out of the way */
-        old_filename = g_strdup_printf ("%s.old", process->priv->log_file);
-        rename (process->priv->log_file, old_filename);
-        g_free (old_filename);
-
-        /* Create new file and log to it */
-        log_fd = g_open (process->priv->log_file, O_WRONLY | O_CREAT | O_TRUNC, 0600);
-        if (log_fd < 0)
-            g_warning ("Failed to open log file %s: %s", process->priv->log_file, g_strerror (errno));
-    }
+        log_fd = log_file_open (process->priv->log_file, process->priv->log_mode);
 
     /* Work out variables to set */
     env_length = g_hash_table_size (process->priv->env);

=== modified file 'src/process.h'
--- src/process.h	2014-09-29 23:42:38 +0000
+++ src/process.h	2015-10-20 10:30:53 +0000
@@ -14,6 +14,8 @@
 
 #include <glib-object.h>
 
+#include "log-mode.h"
+
 G_BEGIN_DECLS
 
 #define PROCESS_TYPE (process_get_type())
@@ -49,7 +51,7 @@
 
 Process *process_new (ProcessRunFunc run_func, gpointer run_func_data);
 
-void process_set_log_file (Process *process, const gchar *path, gboolean log_stdout);
+void process_set_log_file (Process *process, const gchar *path, gboolean log_stdout, LogMode log_mode);
 
 void process_set_clear_environment (Process *process, gboolean clear_environment);
 

=== modified file 'src/seat.c'
--- src/seat.c	2015-10-16 10:20:03 +0000
+++ src/seat.c	2015-10-20 10:30:53 +0000
@@ -602,7 +602,7 @@
         log_filename = g_build_filename (log_dir, filename, NULL);
         g_free (log_dir);
         g_free (filename);
-        session_set_log_file (session, log_filename);
+        session_set_log_file (session, log_filename, LOG_MODE_APPEND);
         g_free (log_filename);
     }
 

=== modified file 'src/session-child.c'
--- src/session-child.c	2015-09-29 20:26:33 +0000
+++ src/session-child.c	2015-10-20 10:30:53 +0000
@@ -26,6 +26,8 @@
 #include "session.h"
 #include "console-kit.h"
 #include "login1.h"
+#include "log-file.h"
+#include "log-mode.h"
 #include "privileges.h"
 #include "x-authority.h"
 #include "configuration.h"
@@ -257,7 +259,8 @@
     int i, version, fd, result;
     gboolean auth_complete = TRUE;
     User *user = NULL;
-    gchar *log_filename, *log_backup_filename = NULL;
+    gchar *log_filename;
+    LogMode log_mode = LOG_MODE_BACKUP_AND_TRUNCATE;
     gsize env_length;
     gsize command_argc;
     gchar **command_argv;
@@ -488,6 +491,8 @@
 
     /* Get the command to run (blocks) */
     log_filename = read_string ();
+    if (version >= 3)
+        read_data (&log_mode, sizeof (log_mode));
     if (version >= 1)
     {
         g_free (tty);
@@ -522,11 +527,9 @@
     /* Redirect stderr to a log file */
     if (log_filename)
     {
-        log_backup_filename = g_strdup_printf ("%s.old", log_filename);
         if (g_path_is_absolute (log_filename))
         {
-            rename (log_filename, log_backup_filename);
-            fd = open (log_filename, O_WRONLY | O_APPEND | O_CREAT, 0600);
+            fd = log_file_open (log_filename, log_mode);
             dup2 (fd, STDERR_FILENO);
             close (fd);
             g_free (log_filename);
@@ -680,8 +683,7 @@
 
         if (log_filename)
         {
-            rename (log_filename, log_backup_filename);
-            fd = open (log_filename, O_WRONLY | O_APPEND | O_CREAT, 0600);
+            fd = log_file_open (log_filename, log_mode);
             if (fd >= 0)
             {
                 dup2 (fd, STDERR_FILENO);

=== modified file 'src/session.c'
--- src/session.c	2015-10-16 10:20:03 +0000
+++ src/session.c	2015-10-20 10:30:53 +0000
@@ -84,6 +84,7 @@
 
     /* File to log to */
     gchar *log_filename;
+    LogMode log_mode;
 
     /* tty this session is running on */
     gchar *tty;
@@ -198,11 +199,12 @@
 }
 
 void
-session_set_log_file (Session *session, const gchar *filename)
+session_set_log_file (Session *session, const gchar *filename, LogMode log_mode)
 {
     g_return_if_fail (session != NULL);
     g_free (session->priv->log_filename);
     session->priv->log_filename = g_strdup (filename);
+    session->priv->log_mode = log_mode;
 }
 
 void
@@ -619,7 +621,7 @@
     close (from_child_input);
 
     /* Indicate what version of the protocol we are using */
-    version = 2;
+    version = 3;
     write_data (session, &version, sizeof (version));
 
     /* Send configuration */
@@ -791,6 +793,7 @@
     if (session->priv->log_filename)
         l_debug (session, "Logging to %s", session->priv->log_filename);
     write_string (session, session->priv->log_filename);
+    write_data (session, &session->priv->log_mode, sizeof (session->priv->log_mode));
     write_string (session, session->priv->tty);
     write_string (session, x_authority_filename);
     g_free (x_authority_filename);
@@ -857,9 +860,11 @@
     if (session_get_is_authenticated (session) && !session->priv->command_run)
     {
         gsize n = 0;
+        LogMode log_mode = LOG_MODE_INVALID;
 
         session->priv->command_run = TRUE;
         write_string (session, NULL); // log filename
+        write_data (session, &log_mode, sizeof (log_mode)); // log mode
         write_string (session, NULL); // tty
         write_string (session, NULL); // xauth filename
         write_string (session, NULL); // xdisplay
@@ -903,6 +908,7 @@
 {
     session->priv = G_TYPE_INSTANCE_GET_PRIVATE (session, SESSION_TYPE, SessionPrivate);
     session->priv->log_filename = g_strdup (".xsession-errors");
+    session->priv->log_mode = LOG_MODE_BACKUP_AND_TRUNCATE;
     session->priv->to_child_input = -1;
     session->priv->from_child_output = -1;
 }

=== modified file 'src/session.h'
--- src/session.h	2014-11-21 07:45:05 +0000
+++ src/session.h	2015-10-20 10:30:53 +0000
@@ -23,6 +23,7 @@
 #include "accounts.h"
 #include "x-authority.h"
 #include "logger.h"
+#include "log-mode.h"
 
 G_BEGIN_DECLS
 
@@ -84,7 +85,7 @@
 
 gboolean session_get_is_guest (Session *session);
 
-void session_set_log_file (Session *session, const gchar *filename);
+void session_set_log_file (Session *session, const gchar *filename, LogMode log_mode);
 
 void session_set_display_server (Session *session, DisplayServer *display_server);
 

=== modified file 'src/unity-system-compositor.c'
--- src/unity-system-compositor.c	2015-10-16 10:04:18 +0000
+++ src/unity-system-compositor.c	2015-10-20 10:30:53 +0000
@@ -407,7 +407,7 @@
 
     /* Setup environment */
     compositor->priv->process = process_new (run_cb, compositor);
-    process_set_log_file (compositor->priv->process, log_file, TRUE);
+    process_set_log_file (compositor->priv->process, log_file, TRUE, LOG_MODE_APPEND);
     g_free (log_file);
     process_set_clear_environment (compositor->priv->process, TRUE);
     process_set_env (compositor->priv->process, "XDG_SEAT", "seat0");

=== modified file 'src/x-server-local.c'
--- src/x-server-local.c	2015-10-16 10:04:18 +0000
+++ src/x-server-local.c	2015-10-20 10:30:53 +0000
@@ -493,7 +493,7 @@
     filename = g_strdup_printf ("%s.log", display_server_get_name (display_server));
     dir = config_get_string (config_get_instance (), "LightDM", "log-directory");
     log_file = g_build_filename (dir, filename, NULL);
-    process_set_log_file (server->priv->x_server_process, log_file, TRUE);
+    process_set_log_file (server->priv->x_server_process, log_file, TRUE, LOG_MODE_APPEND);
     l_debug (display_server, "Logging to %s", log_file);
     g_free (log_file);
     g_free (filename);

=== modified file 'src/x-server-xvnc.c'
--- src/x-server-xvnc.c	2015-10-16 10:20:03 +0000
+++ src/x-server-xvnc.c	2015-10-20 10:30:53 +0000
@@ -201,7 +201,7 @@
     filename = g_strdup_printf ("%s.log", display_server_get_name (display_server));
     dir = config_get_string (config_get_instance (), "LightDM", "log-directory");
     log_file = g_build_filename (dir, filename, NULL);
-    process_set_log_file (server->priv->x_server_process, log_file, FALSE);
+    process_set_log_file (server->priv->x_server_process, log_file, FALSE, LOG_MODE_APPEND);
     l_debug (display_server, "Logging to %s", log_file);
     g_free (log_file);
     g_free (filename);