From 7885e399f12affe1933fcacce7f2dab311ed6761 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Thu, 8 Dec 2011 11:40:08 -0500 Subject: [PATCH] Improve UST channel code Better breakdown of the code when creating/enabling/disabling UST channel. No new features added. It's mostly a refactoring. Signed-off-by: David Goulet --- lttng-sessiond/channel.c | 166 ++++++++++++++++++++++++++++-- lttng-sessiond/channel.h | 7 ++ lttng-sessiond/main.c | 106 +++---------------- lttng/commands/disable_channels.c | 3 +- lttng/commands/enable_channels.c | 5 +- 5 files changed, 185 insertions(+), 102 deletions(-) diff --git a/lttng-sessiond/channel.c b/lttng-sessiond/channel.c index 146807cb2..5a22d2937 100644 --- a/lttng-sessiond/channel.c +++ b/lttng-sessiond/channel.c @@ -15,6 +15,7 @@ * Place - Suite 330, Boston, MA 02111-1307, USA. */ +#define _GNU_SOURCE #include #include @@ -27,6 +28,7 @@ #include "kernel.h" #include "ust-ctl.h" #include "utils.h" +#include "ust-app.h" /* * Return allocated channel attributes. @@ -37,13 +39,13 @@ struct lttng_channel *channel_new_default_attr(int dom) chan = zmalloc(sizeof(struct lttng_channel)); if (chan == NULL) { - perror("zmalloc channel init"); + PERROR("zmalloc channel init"); goto error_alloc; } if (snprintf(chan->name, sizeof(chan->name), "%s", DEFAULT_CHANNEL_NAME) < 0) { - perror("snprintf default channel name"); + PERROR("snprintf default channel name"); goto error; } @@ -90,10 +92,8 @@ int channel_kernel_disable(struct ltt_kernel_session *ksession, goto error; } else if (kchan->enabled == 1) { ret = kernel_disable_channel(kchan); - if (ret < 0) { - if (ret != EEXIST) { - ret = LTTCOMM_KERN_CHAN_DISABLE_FAIL; - } + if (ret < 0 && ret != -EEXIST) { + ret = LTTCOMM_KERN_CHAN_DISABLE_FAIL; goto error; } } @@ -164,3 +164,157 @@ error: free(defattr); return ret; } + +/* + * Enable UST channel for session and domain. + */ +int channel_ust_enable(struct ltt_ust_session *usess, int domain, + struct ltt_ust_channel *uchan) +{ + int ret = LTTCOMM_OK; + + /* If already enabled, everything is OK */ + if (uchan->enabled) { + DBG3("Channel %s already enabled. Skipping", uchan->name); + goto end; + } + + switch (domain) { + case LTTNG_DOMAIN_UST: + DBG2("Channel %s being enabled in UST global domain", uchan->name); + /* Enable channel for global domain */ + ret = ust_app_enable_channel_glb(usess, uchan); + break; + case LTTNG_DOMAIN_UST_PID: + case LTTNG_DOMAIN_UST_PID_FOLLOW_CHILDREN: + case LTTNG_DOMAIN_UST_EXEC_NAME: + ret = LTTCOMM_NOT_IMPLEMENTED; + goto error; + } + + if (ret < 0) { + if (ret != -EEXIST) { + ret = LTTCOMM_UST_CHAN_ENABLE_FAIL; + goto error; + } else { + ret = LTTCOMM_OK; + } + } + + uchan->enabled = 1; + DBG2("Channel %s enabled successfully", uchan->name); + +end: +error: + return ret; +} + +/* + * Create UST channel for session and domain. + */ +int channel_ust_create(struct ltt_ust_session *usess, int domain, + struct lttng_channel *attr) +{ + int ret = LTTCOMM_OK; + struct cds_lfht *chan_ht; + struct ltt_ust_channel *uchan = NULL; + struct lttng_channel *defattr = NULL; + + /* Creating channel attributes if needed */ + if (attr == NULL) { + defattr = channel_new_default_attr(domain); + if (defattr == NULL) { + ret = LTTCOMM_FATAL; + goto error; + } + attr = defattr; + } + + /* Create UST channel */ + uchan = trace_ust_create_channel(attr, usess->pathname); + if (uchan == NULL) { + ret = LTTCOMM_FATAL; + goto error; + } + + switch (domain) { + case LTTNG_DOMAIN_UST: + DBG2("Channel %s being created in UST global domain", uchan->name); + chan_ht = usess->domain_global.channels; + + /* Enable channel for global domain */ + ret = ust_app_create_channel_glb(usess, uchan); + break; + case LTTNG_DOMAIN_UST_PID: + case LTTNG_DOMAIN_UST_PID_FOLLOW_CHILDREN: + case LTTNG_DOMAIN_UST_EXEC_NAME: + default: + ret = LTTCOMM_NOT_IMPLEMENTED; + goto error_free_chan; + } + + if (ret < 0 && ret != -EEXIST) { + ret = LTTCOMM_UST_CHAN_ENABLE_FAIL; + goto error_free_chan; + } + + uchan->enabled = 1; + hashtable_add_unique(chan_ht, &uchan->node); + DBG2("Channel %s created successfully", uchan->name); + + free(defattr); + return LTTCOMM_OK; + +error_free_chan: + trace_ust_destroy_channel(uchan); +error: + free(defattr); + return ret; +} + +/* + * Disable UST channel for session and domain. + */ +int channel_ust_disable(struct ltt_ust_session *usess, int domain, + struct ltt_ust_channel *uchan) +{ + int ret = LTTCOMM_OK; + struct cds_lfht *chan_ht; + + /* Already disabled */ + if (uchan->enabled == 0) { + DBG2("Channel UST %s already disabled", uchan->name); + goto end; + } + + /* Get the right channel's hashtable */ + switch (domain) { + case LTTNG_DOMAIN_UST: + DBG2("Channel %s being disabled in UST global domain", uchan->name); + chan_ht = usess->domain_global.channels; + + /* Disable channel for global domain */ + ret = ust_app_disable_channel_glb(usess, uchan); + break; + case LTTNG_DOMAIN_UST_PID_FOLLOW_CHILDREN: + case LTTNG_DOMAIN_UST_EXEC_NAME: + case LTTNG_DOMAIN_UST_PID: + ret = LTTCOMM_NOT_IMPLEMENTED; + goto error; + } + + if (ret < 0 && ret != -EEXIST) { + ret = LTTCOMM_UST_DISABLE_FAIL; + goto error; + } + + uchan->enabled = 0; + + DBG2("Channel %s disabled successfully", uchan->name); + + return LTTCOMM_OK; + +end: +error: + return ret; +} diff --git a/lttng-sessiond/channel.h b/lttng-sessiond/channel.h index 656a1d526..04052f6d7 100644 --- a/lttng-sessiond/channel.h +++ b/lttng-sessiond/channel.h @@ -32,4 +32,11 @@ int channel_kernel_create(struct ltt_kernel_session *ksession, struct lttng_channel *channel_new_default_attr(int domain); +int channel_ust_create(struct ltt_ust_session *usess, int domain, + struct lttng_channel *attr); +int channel_ust_enable(struct ltt_ust_session *usess, int domain, + struct ltt_ust_channel *uchan); +int channel_ust_disable(struct ltt_ust_session *usess, int domain, + struct ltt_ust_channel *uchan); + #endif /* _LTT_CHANNEL_H */ diff --git a/lttng-sessiond/main.c b/lttng-sessiond/main.c index 822c1df7d..ec45c6676 100644 --- a/lttng-sessiond/main.c +++ b/lttng-sessiond/main.c @@ -2205,29 +2205,20 @@ static int cmd_disable_channel(struct ltt_session *session, case LTTNG_DOMAIN_UST: { struct ltt_ust_channel *uchan; + struct cds_lfht *chan_ht; - /* Get channel in global UST domain HT */ - uchan = trace_ust_find_channel_by_name(usess->domain_global.channels, - channel_name); + chan_ht = usess->domain_global.channels; + + uchan = trace_ust_find_channel_by_name(chan_ht, channel_name); if (uchan == NULL) { ret = LTTCOMM_UST_CHAN_NOT_FOUND; goto error; } - /* Already disabled */ - if (!uchan->enabled) { - DBG2("UST channel %s already disabled", channel_name); - break; - } - - ret = ust_app_disable_channel_glb(usess, uchan); - if (ret < 0) { - ret = LTTCOMM_UST_DISABLE_FAIL; + ret = channel_ust_disable(usess, domain, uchan); + if (ret != LTTCOMM_OK) { goto error; } - - uchan->enabled = 0; - break; } case LTTNG_DOMAIN_UST_PID_FOLLOW_CHILDREN: @@ -2246,40 +2237,6 @@ error: return ret; } -/* - * Copy channel from attributes and set it in the application channel list. - */ -/* -static int copy_ust_channel_to_app(struct ltt_ust_session *usess, - struct lttng_channel *attr, struct ust_app *app) -{ - int ret; - struct ltt_ust_channel *uchan, *new_chan; - - uchan = trace_ust_get_channel_by_key(usess->channels, attr->name); - if (uchan == NULL) { - ret = LTTCOMM_FATAL; - goto error; - } - - new_chan = trace_ust_create_channel(attr, usess->path); - if (new_chan == NULL) { - PERROR("malloc ltt_ust_channel"); - ret = LTTCOMM_FATAL; - goto error; - } - - ret = channel_ust_copy(new_chan, uchan); - if (ret < 0) { - ret = LTTCOMM_FATAL; - goto error; - } - -error: - return ret; -} -*/ - /* * Command LTTNG_ENABLE_CHANNEL processed by the client thread. */ @@ -2288,6 +2245,7 @@ static int cmd_enable_channel(struct ltt_session *session, { int ret; struct ltt_ust_session *usess = session->ust_session; + struct cds_lfht *chan_ht; DBG("Enabling channel %s for session %s", attr->name, session->name); @@ -2316,49 +2274,14 @@ static int cmd_enable_channel(struct ltt_session *session, { struct ltt_ust_channel *uchan; - DBG2("Enabling channel for LTTNG_DOMAIN_UST"); + chan_ht = usess->domain_global.channels; - /* Get channel in global UST domain HT */ - uchan = trace_ust_find_channel_by_name(usess->domain_global.channels, - attr->name); + uchan = trace_ust_find_channel_by_name(chan_ht, attr->name); if (uchan == NULL) { - uchan = trace_ust_create_channel(attr, usess->pathname); - if (uchan == NULL) { - ret = LTTCOMM_UST_CHAN_FAIL; - goto error; - } - - /* Add channel to all registered applications */ - ret = ust_app_create_channel_glb(usess, uchan); - if (ret != 0) { - ret = LTTCOMM_UST_CHAN_FAIL; - goto error; - } - - rcu_read_lock(); - hashtable_add_unique(usess->domain_global.channels, &uchan->node); - rcu_read_unlock(); - - DBG2("UST channel %s added to global domain HT", attr->name); + ret = channel_ust_create(usess, domain, attr); } else { - /* If already enabled, everything is OK */ - if (uchan->enabled) { - break; - } - - ret = ust_app_enable_channel_glb(usess, uchan); - if (ret < 0) { - if (ret != -EEXIST) { - ret = LTTCOMM_UST_CHAN_ENABLE_FAIL; - goto error; - } else { - ret = LTTCOMM_OK; - } - } + ret = channel_ust_enable(usess, domain, uchan); } - - uchan->enabled = 1; - break; } case LTTNG_DOMAIN_UST_PID_FOLLOW_CHILDREN: @@ -2371,8 +2294,6 @@ static int cmd_enable_channel(struct ltt_session *session, goto error; } - ret = LTTCOMM_OK; - error: return ret; } @@ -2629,8 +2550,7 @@ static int cmd_enable_event(struct ltt_session *session, int domain, snprintf(attr->name, NAME_MAX, "%s", channel_name); attr->name[NAME_MAX - 1] = '\0'; - /* Use the internal command enable channel */ - ret = cmd_enable_channel(session, domain, attr); + ret = channel_ust_create(usess, domain, attr); if (ret != LTTCOMM_OK) { free(attr); goto error; @@ -2751,7 +2671,7 @@ static int cmd_enable_event_all(struct ltt_session *session, int domain, attr->name[NAME_MAX - 1] = '\0'; /* Use the internal command enable channel */ - ret = cmd_enable_channel(session, domain, attr); + ret = channel_ust_create(usess, domain, attr); if (ret != LTTCOMM_OK) { free(attr); goto error; diff --git a/lttng/commands/disable_channels.c b/lttng/commands/disable_channels.c index 434aae59a..59ad9fc28 100644 --- a/lttng/commands/disable_channels.c +++ b/lttng/commands/disable_channels.c @@ -111,7 +111,8 @@ static int disable_channels(char *session_name) if (ret < 0) { goto error; } else { - MSG("Channel %s disabled for session %s", channel_name, + MSG("%s channel %s disabled for session %s", + opt_kernel ? "Kernel" : "UST", channel_name, session_name); } diff --git a/lttng/commands/enable_channels.c b/lttng/commands/enable_channels.c index ec4aae3cf..52745f69a 100644 --- a/lttng/commands/enable_channels.c +++ b/lttng/commands/enable_channels.c @@ -181,8 +181,9 @@ static int enable_channel(char *session_name) if (ret < 0) { goto error; } else { - MSG("Channel enabled %s for session %s", - channel_name, session_name); + MSG("%s channel %s enabled for session %s", + opt_kernel ? "Kernel" : "UST", channel_name, + session_name); } /* Next event */ -- 2.34.1