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 <david.goulet@polymtl.ca>

diff --git a/lttng-sessiond/channel.c b/lttng-sessiond/channel.c
index 146807cb21ccedc491734cc3572099e844f47c95..5a22d2937f22a9d95de60f6a7126ff28671402b7 100644 (file)
--- 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 <string.h>
 #include <unistd.h>
 
@@ -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 656a1d5266cfa4fceff7ff2a7279eebc5494b795..04052f6d728592e91ffedd91a48dc355b3073456 100644 (file)
--- 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 822c1df7de1d86b5fefcc42d2f5a99e5564202d8..ec45c6676bed50b8934287130011a93b55fef0f8 100644 (file)
--- 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 434aae59a590d679ddd6db4138bb43368cd963c0..59ad9fc28f97929526951df06f7b8bcd7d69cb09 100644 (file)
--- 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 ec4aae3cf44e168e8d6e8c7f1152c45e4caf472f..52745f69a651d61ec91f18ca1b8cff81bf0c9d6a 100644 (file)
--- 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 */