[lxc-devel] [lxc/master] cleanup monitor + improve log
brauner on Github
lxc-bot at linuxcontainers.org
Sat Nov 26 04:31:26 UTC 2016
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 352 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20161126/54616071/attachment.bin>
-------------- next part --------------
From 292b1d177a331837c0944f5c2aaf36adb82a8993 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 26 Nov 2016 05:03:55 +0100
Subject: [PATCH 1/2] monitor: non-functional changes
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/monitor.c | 147 +++++++++++++++++++++++++++++++-----------------------
1 file changed, 84 insertions(+), 63 deletions(-)
diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index dd8ca9e..fab6f55 100644
--- a/src/lxc/monitor.c
+++ b/src/lxc/monitor.c
@@ -22,30 +22,31 @@
* Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
*/
-#include <stdio.h>
#include <errno.h>
-#include <unistd.h>
-#include <string.h>
-#include <stdlib.h>
-#include <stddef.h>
#include <fcntl.h>
#include <inttypes.h>
+#include <poll.h>
+#include <stddef.h>
#include <stdint.h>
-#include <sys/types.h>
-#include <sys/stat.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <net/if.h>
+#include <netinet/in.h>
#include <sys/param.h>
#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/types.h>
#include <sys/wait.h>
-#include <netinet/in.h>
-#include <net/if.h>
-#include <poll.h>
-#include "error.h"
+#include "config.h"
#include "af_unix.h"
+#include "error.h"
#include "log.h"
#include "lxclock.h"
-#include "state.h"
#include "monitor.h"
+#include "state.h"
#include "utils.h"
lxc_log_define(lxc_monitor, lxc);
@@ -63,21 +64,21 @@ int lxc_monitor_fifo_name(const char *lxcpath, char *fifo_path, size_t fifo_path
if (do_mkdirp) {
ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s", rundir, lxcpath);
- if (ret < 0 || ret >= fifo_path_sz) {
- ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath);
+ if (ret < 0 || (size_t)ret >= fifo_path_sz) {
+ ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo.", rundir, lxcpath);
free(rundir);
return -1;
}
ret = mkdir_p(fifo_path, 0755);
if (ret < 0) {
- ERROR("unable to create monitor fifo dir %s", fifo_path);
+ ERROR("Unable to create monitor fifo directory %s.", fifo_path);
free(rundir);
return ret;
}
}
ret = snprintf(fifo_path, fifo_path_sz, "%s/lxc/%s/monitor-fifo", rundir, lxcpath);
- if (ret < 0 || ret >= fifo_path_sz) {
- ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo", rundir, lxcpath);
+ if (ret < 0 || (size_t)ret >= fifo_path_sz) {
+ ERROR("rundir/lxcpath (%s/%s) too long for monitor fifo.", rundir, lxcpath);
free(rundir);
return -1;
}
@@ -96,14 +97,18 @@ static void lxc_monitor_fifo_send(struct lxc_msg *msg, const char *lxcpath)
if (ret < 0)
return;
- /* open the fifo nonblock in case the monitor is dead, we don't want
- * the open to wait for a reader since it may never come.
+ /* Open the fifo nonblock in case the monitor is dead, we don't want the
+ * open to wait for a reader since it may never come.
*/
- fd = open(fifo_path, O_WRONLY|O_NONBLOCK);
+ fd = open(fifo_path, O_WRONLY | O_NONBLOCK);
if (fd < 0) {
- /* it is normal for this open to fail ENXIO when there is no
- * monitor running, so we don't log it
+ /* It is normal for this open() to fail with ENXIO when there is
+ * no monitor running, so we don't log it.
*/
+ if (errno == ENXIO)
+ return;
+
+ WARN("Failed to open fifo to send message: %s.", strerror(errno));
return;
}
@@ -115,34 +120,33 @@ static void lxc_monitor_fifo_send(struct lxc_msg *msg, const char *lxcpath)
ret = write(fd, msg, sizeof(*msg));
if (ret != sizeof(*msg)) {
close(fd);
- SYSERROR("failed to write monitor fifo %s", fifo_path);
+ SYSERROR("Failed to write to monitor fifo \"%s\".", fifo_path);
return;
}
close(fd);
}
-void lxc_monitor_send_state(const char *name, lxc_state_t state, const char *lxcpath)
+void lxc_monitor_send_state(const char *name, lxc_state_t state,
+ const char *lxcpath)
{
- struct lxc_msg msg = { .type = lxc_msg_state,
- .value = state };
+ struct lxc_msg msg = {.type = lxc_msg_state, .value = state};
strncpy(msg.name, name, sizeof(msg.name));
msg.name[sizeof(msg.name) - 1] = 0;
lxc_monitor_fifo_send(&msg, lxcpath);
}
-void lxc_monitor_send_exit_code(const char *name, int exit_code, const char *lxcpath)
+void lxc_monitor_send_exit_code(const char *name, int exit_code,
+ const char *lxcpath)
{
- struct lxc_msg msg = { .type = lxc_msg_exit_code,
- .value = exit_code };
+ struct lxc_msg msg = {.type = lxc_msg_exit_code, .value = exit_code};
strncpy(msg.name, name, sizeof(msg.name));
msg.name[sizeof(msg.name) - 1] = 0;
lxc_monitor_fifo_send(&msg, lxcpath);
}
-
/* routines used by monitor subscribers (lxc-monitor) */
int lxc_monitor_close(int fd)
{
@@ -152,20 +156,22 @@ int lxc_monitor_close(int fd)
int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) {
size_t len;
int ret;
- char *sockname = &addr->sun_path[1];
+ char *sockname;
char *path;
uint64_t hash;
/* addr.sun_path is only 108 bytes, so we hash the full name and
* then append as much of the name as we can fit.
*/
+ sockname = &addr->sun_path[1];
memset(addr, 0, sizeof(*addr));
addr->sun_family = AF_UNIX;
+
len = strlen(lxcpath) + 18;
path = alloca(len);
ret = snprintf(path, len, "lxc/%s/monitor-sock", lxcpath);
- if (ret < 0 || ret >= len) {
- ERROR("memory error creating monitor path");
+ if (ret < 0 || (size_t)ret >= len) {
+ ERROR("Failed to create path for monitor.");
return -1;
}
@@ -174,24 +180,27 @@ int lxc_monitor_sock_name(const char *lxcpath, struct sockaddr_un *addr) {
ret = snprintf(sockname, len, "lxc/%016" PRIx64 "/%s", hash, lxcpath);
if (ret < 0)
return -1;
+
sockname[sizeof(addr->sun_path)-3] = '\0';
- INFO("using monitor sock name %s", sockname);
+ INFO("Using monitor socket name \"%s\".", sockname);
+
return 0;
}
int lxc_monitor_open(const char *lxcpath)
{
struct sockaddr_un addr;
- int fd,ret = 0;
- int retry,backoff_ms[] = {10, 50, 100};
+ int fd;
+ size_t retry;
size_t len;
+ int ret = 0, backoff_ms[] = {10, 50, 100};
if (lxc_monitor_sock_name(lxcpath, &addr) < 0)
return -1;
fd = socket(PF_UNIX, SOCK_STREAM, 0);
if (fd < 0) {
- ERROR("socket : %s", strerror(errno));
+ ERROR("Failed to create socket: %s.", strerror(errno));
return -1;
}
@@ -199,23 +208,25 @@ int lxc_monitor_open(const char *lxcpath)
if (len >= sizeof(addr.sun_path) - 1) {
ret = -1;
errno = ENAMETOOLONG;
- goto err1;
+ goto on_error;
}
- for (retry = 0; retry < sizeof(backoff_ms)/sizeof(backoff_ms[0]); retry++) {
+ for (retry = 0; retry < sizeof(backoff_ms) / sizeof(backoff_ms[0]); retry++) {
ret = connect(fd, (struct sockaddr *)&addr, offsetof(struct sockaddr_un, sun_path) + len);
if (ret == 0 || errno != ECONNREFUSED)
break;
- ERROR("connect : backing off %d", backoff_ms[retry]);
+ ERROR("Failed to connect to monitor socket. Retrying in %d ms.", backoff_ms[retry]);
usleep(backoff_ms[retry] * 1000);
}
if (ret < 0) {
- ERROR("connect : %s", strerror(errno));
- goto err1;
+ ERROR("Failed to connect to monitor socket: %s.", strerror(errno));
+ goto on_error;
}
+
return fd;
-err1:
+
+on_error:
close(fd);
return ret;
}
@@ -232,22 +243,23 @@ int lxc_monitor_read_fdset(struct pollfd *fds, nfds_t nfds, struct lxc_msg *msg,
else if (ret == 0)
return -2; // timed out
- /* only read from the first ready fd, the others will remain ready
- * for when this routine is called again
+ /* Only read from the first ready fd, the others will remain ready for
+ * when this routine is called again.
*/
for (i = 0; i < nfds; i++) {
if (fds[i].revents != 0) {
fds[i].revents = 0;
ret = recv(fds[i].fd, msg, sizeof(*msg), 0);
if (ret <= 0) {
- SYSERROR("client failed to recv (monitord died?) %s",
- strerror(errno));
+ SYSERROR("Failed to receive message. Did monitord die?: %s.", strerror(errno));
return -1;
}
return ret;
}
}
- SYSERROR("no ready fd found?");
+
+ SYSERROR("No ready fd found.");
+
return -1;
}
@@ -267,72 +279,81 @@ int lxc_monitor_read(int fd, struct lxc_msg *msg)
return lxc_monitor_read_timeout(fd, msg, -1);
}
-
#define LXC_MONITORD_PATH LIBEXECDIR "/lxc/lxc-monitord"
-/* used to spawn a monitord either on startup of a daemon container, or when
- * lxc-monitor starts
+/* Used to spawn a monitord either on startup of a daemon container, or when
+ * lxc-monitor starts.
*/
int lxc_monitord_spawn(const char *lxcpath)
{
- pid_t pid1,pid2;
+ pid_t pid1, pid2;
int pipefd[2];
char pipefd_str[11];
- char * const args[] = {
- LXC_MONITORD_PATH,
- (char *)lxcpath,
- pipefd_str,
- NULL,
+ char *const args[] = {
+ LXC_MONITORD_PATH,
+ (char *)lxcpath,
+ pipefd_str,
+ NULL,
};
/* double fork to avoid zombies when monitord exits */
pid1 = fork();
if (pid1 < 0) {
- SYSERROR("failed to fork");
+ SYSERROR("Failed to fork().");
return -1;
}
if (pid1) {
+ DEBUG("Going to wait for pid %d.", pid1);
if (waitpid(pid1, NULL, 0) != pid1)
return -1;
return 0;
}
if (pipe(pipefd) < 0) {
- SYSERROR("failed to create pipe");
+ SYSERROR("Failed to create pipe.");
exit(EXIT_FAILURE);
}
pid2 = fork();
if (pid2 < 0) {
- SYSERROR("failed to fork");
+ SYSERROR("Failed to fork().");
exit(EXIT_FAILURE);
}
+
if (pid2) {
char c;
- /* wait for daemon to create socket */
+ /* Wait for daemon to create socket. */
close(pipefd[1]);
- /* sync with child, we're ignoring the return from read
+
+ /* Sync with child, we're ignoring the return from read
* because regardless if it works or not, either way we've
* synced with the child process. the if-empty-statement
* construct is to quiet the warn-unused-result warning.
*/
if (read(pipefd[0], &c, 1))
;
+
close(pipefd[0]);
+
exit(EXIT_SUCCESS);
}
if (setsid() < 0) {
- SYSERROR("failed to setsid");
+ SYSERROR("Failed to setsid().");
exit(EXIT_FAILURE);
}
+
lxc_check_inherited(NULL, true, pipefd[1]);
if (null_stdfds() < 0)
exit(EXIT_FAILURE);
+
close(pipefd[0]);
+
sprintf(pipefd_str, "%d", pipefd[1]);
+
execvp(args[0], args);
+
exit(EXIT_FAILURE);
}
From 487b14b6ae39f05d5ba32085f7192ea381ca6fa0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sat, 26 Nov 2016 05:28:58 +0100
Subject: [PATCH 2/2] monitor: log which pipe fd is currently used
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/monitor.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/src/lxc/monitor.c b/src/lxc/monitor.c
index fab6f55..1afaa5a 100644
--- a/src/lxc/monitor.c
+++ b/src/lxc/monitor.c
@@ -284,11 +284,13 @@ int lxc_monitor_read(int fd, struct lxc_msg *msg)
/* Used to spawn a monitord either on startup of a daemon container, or when
* lxc-monitor starts.
*/
+#define __INT_LEN 21
int lxc_monitord_spawn(const char *lxcpath)
{
- pid_t pid1, pid2;
+ int ret;
int pipefd[2];
- char pipefd_str[11];
+ char pipefd_str[__INT_LEN];
+ pid_t pid1, pid2;
char *const args[] = {
LXC_MONITORD_PATH,
@@ -308,6 +310,7 @@ int lxc_monitord_spawn(const char *lxcpath)
DEBUG("Going to wait for pid %d.", pid1);
if (waitpid(pid1, NULL, 0) != pid1)
return -1;
+ DEBUG("Finished waiting on pid %d.", pid1);
return 0;
}
@@ -351,7 +354,11 @@ int lxc_monitord_spawn(const char *lxcpath)
close(pipefd[0]);
- sprintf(pipefd_str, "%d", pipefd[1]);
+ ret = snprintf(pipefd_str, __INT_LEN, "%d", pipefd[1]);
+ if (ret < 0 || ret >= __INT_LEN)
+ exit(EXIT_FAILURE);
+
+ DEBUG("Using pipe file descriptor %d for monitord.", pipefd[1]);
execvp(args[0], args);
More information about the lxc-devel
mailing list