Enable -Wformat plus additional format checks. Currently equivalent to
-Wformat -Wformat-nonliteral -Wformat-security -Wformat-y2k.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
This reverts commit 2fb7cf0b32.
The problem wasn't caused by the reverted commit and was fixed in
commit 0c9b1f826d ("macro: calculate buffer lengths correctly")
The full explanation can be taken from the following irc excerpt from
the #lxc-dev channel:
│19:54:47 brauner | there was a bug in one of the standard macros we used
│19:55:01 brauner | and the changes by INTTYPE_TO_STRLEN() caused the issue to surface
│19:55:03 brauner | which is good
│19:55:16 brauner | i sent a branch and stgraber merged it that fixes it
│19:57:56 Blub\0 | so...
│19:58:31 Blub\0 | still doesn't explain how it was the sizeof() patch
│20:07:14 brauner | Blub\0: so here's the long explanation
│20:07:35 brauner | Blub\0: stgraber bumped pid_max on our jenkins test builders
│20:07:53 brauner | Blub\0: because we're running *a lot* of containers
│20:07:56 brauner | in any case
│20:08:06 brauner | there was a buffer
│20:08:12 brauner | LXC_LSMATTRLEN
│20:08:59 brauner | it used to be
│20:09:03 brauner | -/* /proc/pid-to-str/attr/current = (5 + INTTYPE_TO_STRLEN(pid_t) + 7 + 1) */
│20:09:03 brauner | -#define LXC_LSMATTRLEN (5 + INTTYPE_TO_STRLEN(pid_t) + 7 + 1)
│20:09:14 brauner | which one can see is wrong
│20:09:21 brauner | before the INTTYPE patchset
│20:09:40 brauner | INTTYPE_TO_STRLEN(pid_t) was LXC_NUMSTRLEN64
│20:09:45 brauner | which gave you 21 chars
│20:09:57 brauner | so it accounted for the missing parts
│20:10:03 brauner | because the correct macro should've been
│20:10:17 brauner | +/* /proc/ = 6
│20:10:17 brauner | + * +
│20:10:17 brauner | + * <pid-as-str> = INTTYPE_TO_STRLEN(pid_t)
│20:10:17 brauner | + * +
│20:10:17 brauner | + * /attr/ = 6
│20:10:17 brauner | + * +
│20:10:17 brauner | + * /current = 8
│20:10:17 brauner | + * +
│20:10:17 brauner | + * \0 = 1
│20:10:17 brauner | + */
│20:10:17 brauner | +#define LXC_LSMATTRLEN (6 + INTTYPE_TO_STRLEN(pid_t) + 6 + 8 + 1)
│20:10:24 Blub\0 | still
│20:10:31 brauner | the issue was only seen
│20:10:39 brauner | when the pid number hit a specific maximum
│20:10:50 Blub\0 | the sizeof patch only changed instances of actual char buf[A_FIXED_NUMBER] + snprintf(buf, A_FIXED_NUMBER, ...)
│20:10:54 brauner | aka exceeded the newly shortened buffer
│20:11:42 brauner | your patch was a red herring
│20:12:03 Blub\0 | I guess
│20:12:06 brauner | it didn't cause it
│20:12:14 brauner | it just surfaced at the same time it was merged
│20:12:25 Blub\0 | so we can revert the revert then? :)
│20:12:35 brauner | yes, that was th eplan all along
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
This reverts commit 81a3bb64b4.
This commit broke all builders running with pid_max > 32768.
Reverting for now so we can bring the build farm back online.
Signed-off-by: Stéphane Graber <stgraber@ubuntu.com>
We don't want to link log.{c,h} against utils.{c,h} for the sake of our static
builds init.lxc.static. This means lxc_write_nointr() will not be available. So
handle it EINTR.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
When using the LXC API multi-thread and users initialize a log:
struct lxc_log log;
log.name = "my-log";
lxc_log_init(&log);
all threads will have the same "my-log" prefix even though thy might call
lxc_container_new() in separate threads. There is currently no easy way to
handle per-thread container name prefixes.
To handle this carry a reference to the name of the container in struct
lxc_conf and if no log.name was set, use it by default. This way each thread
will get the container it is currently working on as a log-prefix.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Reported-by: duguhaotian <duguhaotian@gmail.com>
This has annoyed me for a long time, 3.0 seems like the time to fix it :).
I think the way that the log prefix was intended to be used was perhaps a
dynamic prefix per file, but we don't do that today; we include the
filename later in the log message. Instead, we use it as the tool name,
which for liblxc is always "lxc", but could also be things like
"lxc-cgroup" or whatever. There is absolutely no reason to pad this, since
it is always the same for every log file (in fact, we could probably get
rid of the prefix all together, but that seems slightly more drastic).
Instead, let's just drop this padding. Hopefully this will save thousands
of hours of slight annoyance and right scrolling in various pastebins.
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
This caused the linked list of appenders to loop on itself, creating
an infinite logging loop in `__lxc_log_append`.
Signed-off-by: Felix Abecassis <fabecassis@nvidia.com>
* rename lxc.logfile to lxc.log.file
* renaem lxc.loglevel to lxc.log.level
* rename lxc.syslog to lxc.log.syslog
the legacy keys will be kept around until LXC 3.0 and then will be
removed.
Signed-off-by: 0x0916 <w@laoqinren.net>
When we merged the new logging function for the api we exposed the log level
argument in the struct as "priority" which we actually requested to be changed
to "level" which somehow didn't happen and we missed it. Given the fact there
has been no new liblxc release let's fix it right now before it hits users.
Also, take the chance to change the terminology in the log from "priority" to
"level" globally. This is to prevent confusion with syslog's "priority"
argument which we also support.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
The conversion algorithm used uses a clever trick by letting a year start at 1
March. So we need to add 1 for January and February.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
- single digit months, days, hours, minutes, and seconds should always be
preceded by a 0.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
This allows us to generate nice timestamps in a thread-safe manner without
relying on locale touching functions from any libc.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Converts a unix time Epoch given by a struct timespec to a UTC string useable
in our logging functions. Maybe expanded to allow for more generic formatting.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Our log functions need to make extra sure that they are thread-safe. We had
some problems with that before. This especially involves time-conversion
functions. I don't want to find any localtime() or gmtime() functions or
relatives in here. Not even localtime_r() or gmtime_r() or relatives. They all
fiddle with global variables and locking in various libcs. They cause deadlocks
when liblxc is used multi-threaded and no matter how smart you think you are,
you __will__ cause trouble using them.
(As a short example how this can cause trouble: LXD uses forkstart to fork off
a new process that runs the container. At the same time the go runtime LXD
relies on does its own multi-threading thing which we can't control. The
fork()ing + threading then seems to mess with the locking states in these time
functions causing deadlocks.)
The current solution is to be good old unix people and use the Epoch as our
reference point and simply use the seconds and nanoseconds that have past since
then. This relies on clock_gettime() which is explicitly marked MT-Safe with no
restrictions! This way, anyone who is really strongly invested in getting the
actual time the log entry was created, can just convert it for themselves. Our
logging is mostly done for debugging purposes so don't try to make it pretty.
Pretty might cost you thread-safety.
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
localtime_r() can lead to deadlocks because it calls __tzset() and
__tzconvert() internally. The deadlock stems from an interaction between these
functions and the functions in monitor.c and commands.{c,h}. The latter
functions will write to the log independent of the container thread that is
currently running. Since the monitor fork()ed it seems to duplicate the mutex
states of the time functions mentioned above causing the deadlock.
As a short termm fix, I suggest to simply disable receiving the time when
monitor.c or command.{c,h} functions are called. This should be ok, since the
[lxc monitor] will only emit a few messages and thread-safety is currently more
important than beautiful logs. The rest of the log stays the same as it was
before.
Here is an example output from logs where I printed the pid and tid of the
process that is currently writing to the log:
lxc 20161125170200.619 INFO lxc_start: 18695-18695: - start.c:lxc_check_inherited:243 - Closed inherited fd: 23.
lxc 20161125170200.640 DEBUG lxc_start: 18677-18677: - start.c:__lxc_start:1334 - Not dropping CAP_SYS_BOOT or watching utmp.
lxc 20161125170200.640 INFO lxc_cgroup: 18677-18677: - cgroups/cgroup.c:cgroup_init:68 - cgroup driver cgroupfs-ng initing for lxc-test-concurrent-0
----------> lxc 20150427012246.000 INFO lxc_monitor: 13017-18622: - monitor.c:lxc_monitor_sock_name:178 - using monitor sock name lxc/ad055575fe28ddd5//var/lib/lxc
lxc 20161125170200.662 DEBUG lxc_cgfsng: 18677-18677: - cgroups/cgfsng.c:filter_and_set_cpus:478 - No isolated cpus detected.
lxc 20161125170200.662 DEBUG lxc_cgfsng: 18677-18677: - cgroups/cgfsng.c:handle_cpuset_hierarchy:648 - "cgroup.clone_children" was already set to "1".
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
