lttng add-trigger: replace event rule type options with --type

Implement the proposal to replace the --function, --probe,
--userspace-probe, --syscall and --tracepoint options, used to select an
event rule type, with a single --type option.

The --probe, --userspace-probe and --function options used to take an
argument, the name of the location.  For these types, the --location
option is introduced to specify that information.

Change-Id: Ib62e502beb6832a3d26cb135ca6b2746382f2075
Signed-off-by: Simon Marchi <simon.marchi@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng/commands/add_trigger.c b/src/bin/lttng/commands/add_trigger.c
index b976ffce92432eb269455aec266d13e67b246513..1859a7fa8c46fcd0172ef14a3c2a5cfca18bec65 100644 (file)
--- a/src/bin/lttng/commands/add_trigger.c
+++ b/src/bin/lttng/commands/add_trigger.c
@@ -51,12 +51,8 @@ enum {
        OPT_LOGLEVEL_ONLY,
 
        OPT_DOMAIN,
-
-       OPT_FUNCTION,
-       OPT_PROBE,
-       OPT_USERSPACE_PROBE,
-       OPT_SYSCALL,
-       OPT_TRACEPOINT,
+       OPT_TYPE,
+       OPT_LOCATION,
 
        OPT_MAX_SIZE,
        OPT_DATA_URL,
@@ -76,13 +72,8 @@ static const struct argpar_opt_descr event_rule_opt_descrs[] = {
        { OPT_EVENT_NAME, 'E', "event-name", true },
 
        { OPT_DOMAIN, 'd', "domain", true },
-
-       /* Event rule types */
-       { OPT_FUNCTION, '\0', "function", true },
-       { OPT_PROBE, '\0', "probe", true },
-       { OPT_USERSPACE_PROBE, '\0', "userspace-probe", true },
-       { OPT_SYSCALL, '\0', "syscall" },
-       { OPT_TRACEPOINT, '\0', "tracepoint" },
+       { OPT_TYPE, 't', "type", true },
+       { OPT_LOCATION, 'L', "location", true },
 
        /* Capture descriptor */
        { OPT_CAPTURE, '\0', "capture", true },
@@ -126,19 +117,37 @@ end:
 }
 
 static
-bool assign_event_rule_type(enum lttng_event_rule_type *dest,
-               enum lttng_event_rule_type src)
+bool assign_event_rule_type(enum lttng_event_rule_type *dest, const char *arg)
 {
        bool ret;
 
-       if (*dest == LTTNG_EVENT_RULE_TYPE_UNKNOWN || *dest == src) {
-               *dest = src;
-               ret = true;
+       if (*dest != LTTNG_EVENT_RULE_TYPE_UNKNOWN) {
+               ERR("More than one `--type` was specified.");
+               goto error;
+       }
+
+       if (strcmp(arg, "tracepoint") == 0 || strcmp(arg, "logging") == 0) {
+               *dest = LTTNG_EVENT_RULE_TYPE_TRACEPOINT;
+       } else if (strcmp (arg, "kprobe") == 0 || strcmp(arg, "kernel-probe") == 0) {
+               *dest = LTTNG_EVENT_RULE_TYPE_KERNEL_PROBE;
+       } else if (strcmp (arg, "uprobe") == 0 || strcmp(arg, "userspace-probe") == 0) {
+               *dest = LTTNG_EVENT_RULE_TYPE_USERSPACE_PROBE;
+       } else if (strcmp (arg, "function") == 0) {
+               *dest = LTTNG_EVENT_RULE_TYPE_KERNEL_FUNCTION;
+       } else if (strcmp (arg, "syscall") == 0) {
+               *dest = LTTNG_EVENT_RULE_TYPE_SYSCALL;
        } else {
-               ERR("Multiple event types specified.");
-               ret = false;
+               ERR("Invalid `--type` value: %s", arg);
+               goto error;
        }
 
+       ret = true;
+       goto end;
+
+error:
+       ret = false;
+
+end:
        return ret;
 }
 
@@ -557,9 +566,9 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
        char *exclude_names = NULL;
        char **exclusion_list = NULL;
 
-       /* Holds the argument of --probe / --userspace-probe. */
-       char *probe_source = NULL;
-       char *probe_event_name = NULL;
+       /* For userspace / kernel probe and function. */
+       char *location = NULL;
+       char *event_name = NULL;
 
        /* Filter. */
        char *filter = NULL;
@@ -600,65 +609,34 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
                        switch (item_opt->descr->id) {
                        /* Domains. */
                        case OPT_DOMAIN:
-                               if (!assign_domain_type(&domain_type, item_opt->arg)) {
+                               if (!assign_domain_type(&domain_type,
+                                               item_opt->arg)) {
                                        goto error;
                                }
 
                                break;
-
-                       /* Event rule types */
-                       case OPT_FUNCTION:
+                       case OPT_TYPE:
                                if (!assign_event_rule_type(&event_rule_type,
-                                               LTTNG_EVENT_RULE_TYPE_KERNEL_FUNCTION)) {
+                                               item_opt->arg)) {
                                        goto error;
                                }
 
                                break;
-                       case OPT_PROBE:
-                               if (!assign_event_rule_type(&event_rule_type,
-                                               LTTNG_EVENT_RULE_TYPE_KERNEL_PROBE)) {
-                                       goto error;
-                               }
-
-                               if (!assign_string(&probe_source, item_opt->arg,
-                                                   "--probe")) {
-                                       goto error;
-                               }
-
-                               break;
-                       case OPT_USERSPACE_PROBE:
-                               if (!assign_event_rule_type(&event_rule_type,
-                                               LTTNG_EVENT_RULE_TYPE_USERSPACE_PROBE)) {
-                                       goto error;
-                               }
-
-                               if (!assign_string(&probe_source, item_opt->arg,
-                                                   "--userspace-probe")) {
+                       case OPT_LOCATION:
+                               if (!assign_string(&location,
+                                               item_opt->arg,
+                                               "--location/-L")) {
                                        goto error;
                                }
 
                                break;
                        case OPT_EVENT_NAME:
-                               if (!assign_string(&probe_event_name,
+                               if (!assign_string(&event_name,
                                                    item_opt->arg,
                                                    "--event-name/-E")) {
                                        goto error;
                                }
 
-                               break;
-                       case OPT_SYSCALL:
-                               if (!assign_event_rule_type(&event_rule_type,
-                                               LTTNG_EVENT_RULE_TYPE_SYSCALL)) {
-                                       goto error;
-                               }
-
-                               break;
-                       case OPT_TRACEPOINT:
-                               if (!assign_event_rule_type(&event_rule_type,
-                                               LTTNG_EVENT_RULE_TYPE_TRACEPOINT)) {
-                                       goto error;
-                               }
-
                                break;
                        case OPT_FILTER:
                                if (!assign_string(&filter, item_opt->arg,
@@ -779,20 +757,36 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
        }
 
        /*
+        * Option --location is only applicable to (and mandatory for) event
+        * rules of type {k,u}probe and function.
+        *
         * Option --event-name is only applicable to event rules of type probe.
-        * If omitted, it defaults to the probe location.
+        * If omitted, it defaults to the location.
         */
        switch (event_rule_type) {
        case LTTNG_EVENT_RULE_TYPE_KERNEL_PROBE:
        case LTTNG_EVENT_RULE_TYPE_USERSPACE_PROBE:
-               if (!probe_event_name) {
-                       probe_event_name = strdup(probe_source);
+       case LTTNG_EVENT_RULE_TYPE_KERNEL_FUNCTION:
+               if (!location) {
+                       ERR("Event rule of type %s requires a --location.",
+                       lttng_event_rule_type_str(event_rule_type));
+                       goto error;
+               }
+
+               if (!event_name) {
+                       event_name = strdup(location);
                }
 
                break;
 
        default:
-               if (probe_event_name) {
+               if (location) {
+                       ERR("Can't use --location with %s event rules.",
+                       lttng_event_rule_type_str(event_rule_type));
+                       goto error;
+               }
+
+               if (event_name) {
                        ERR("Can't use --event-name with %s event rules.",
                                        lttng_event_rule_type_str(
                                                        event_rule_type));
@@ -964,7 +958,7 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
                enum lttng_event_rule_status event_rule_status;
 
                ret = parse_kernel_probe_opts(
-                               probe_source, &kernel_probe_location);
+                               location, &kernel_probe_location);
                if (ret) {
                        ERR("Failed to parse kernel probe location.");
                        goto error;
@@ -979,10 +973,10 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
 
                event_rule_status =
                                lttng_event_rule_kernel_probe_set_event_name(
-                                               res.er, probe_event_name);
+                                               res.er, event_name);
                if (event_rule_status != LTTNG_EVENT_RULE_STATUS_OK) {
                        ERR("Failed to set kprobe event rule's name to '%s'.",
-                                       probe_event_name);
+                                       event_name);
                        goto error;
                }
 
@@ -994,7 +988,7 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
                enum lttng_event_rule_status event_rule_status;
 
                ret = parse_userspace_probe_opts(
-                               probe_source, &userspace_probe_location);
+                               location, &userspace_probe_location);
                if (ret) {
                        ERR("Failed to parse user space probe location.");
                        goto error;
@@ -1008,10 +1002,10 @@ struct parse_event_rule_res parse_event_rule(int *argc, const char ***argv)
 
                event_rule_status =
                                lttng_event_rule_userspace_probe_set_event_name(
-                                               res.er, probe_event_name);
+                                               res.er, event_name);
                if (event_rule_status != LTTNG_EVENT_RULE_STATUS_OK) {
                        ERR("Failed to set user space probe event rule's name to '%s'.",
-                                       probe_event_name);
+                                       event_name);
                        goto error;
                }
 
@@ -1072,8 +1066,8 @@ end:
        free(name);
        free(exclude_names);
        free(loglevel_str);
-       free(probe_source);
-       free(probe_event_name);
+       free(location);
+       free(event_name);
 
        strutils_free_null_terminated_array_of_strings(exclusion_list);
        lttng_kernel_probe_location_destroy(kernel_probe_location);

diff --git a/tests/regression/tools/trigger/test_add_trigger_cli b/tests/regression/tools/trigger/test_add_trigger_cli
index 7b56fcfa603263ff1f6746572094fc81d7b65112..207689477ace87c3d6155e6f5af81c00b46de616 100755 (executable)
--- a/tests/regression/tools/trigger/test_add_trigger_cli
+++ b/tests/regression/tools/trigger/test_add_trigger_cli
@@ -23,7 +23,7 @@ TESTDIR="$CURDIR/../../.."
 # shellcheck source=../../../utils/utils.sh
 source "$TESTDIR/utils/utils.sh"
 
-plan_tests 228
+plan_tests 252
 
 FULL_LTTNG_BIN="${TESTDIR}/../src/bin/lttng/${LTTNG_BIN}"
 
@@ -126,57 +126,61 @@ test_success "rotate session action polices" \
        --action rotate-session my_session \
        --rate-policy=once-after:55
 
-skip $ist_root "non-root user: skipping kprobe tests" 9 || {
-       test_success "--condition event-rule-matches probe by symbol" \
-               --condition event-rule-matches --domain=kernel --probe=lttng_channel_enable --event-name=my_channel_enable \
-               --action notify
-
-       channel_enable_addr=$(grep ' t lttng_channel_enable\s\[lttng_tracer\]$' /proc/kallsyms | cut -f 1 -d ' ')
-       channel_disable_addr=$(grep ' t lttng_channel_disable\s\[lttng_tracer\]$' /proc/kallsyms | cut -f 1 -d ' ')
-
-       # We need to find a valid offset.
-       base_symbol=""
-       offset=0
-       if [[ 0x$channel_enable_addr -lt 0x$channel_disable_addr ]]; then
-               base_symbol="lttng_channel_enable"
-               offset=$(( 0x$channel_disable_addr - 0x$channel_enable_addr ))
-       else
-               base_symbol="lttng_channel_disable"
-               offset=$(( 0x$channel_enable_addr - 0x$channel_disable_addr ))
-       fi
-
-       offset_hex="0x$(printf '%x' $offset)"
-
-       test_success "--condition event-rule-matches probe by symbol with offset" \
-               --condition event-rule-matches --domain=kernel --probe="${base_symbol}+${offset_hex}" --event-name=my_$base_symbol \
-               --action notify
-
-       test_success "--condition event-rule-matches probe by address" \
-               --condition event-rule-matches --domain=kernel "--probe=0x${channel_enable_addr}" --event-name=my_channel_enable \
-               --action notify
+skip $ist_root "non-root user: skipping kprobe tests" 18 || {
+       for type in kprobe kernel-probe; do
+               test_success "--condition event-rule-matches probe by symbol" \
+                       --condition event-rule-matches --domain=kernel --type=$type --location=lttng_channel_enable --event-name=my_channel_enable \
+                       --action notify
+
+               channel_enable_addr=$(grep ' t lttng_channel_enable\s\[lttng_tracer\]$' /proc/kallsyms | cut -f 1 -d ' ')
+               channel_disable_addr=$(grep ' t lttng_channel_disable\s\[lttng_tracer\]$' /proc/kallsyms | cut -f 1 -d ' ')
+
+               # We need to find a valid offset.
+               base_symbol=""
+               offset=0
+               if [[ 0x$channel_enable_addr -lt 0x$channel_disable_addr ]]; then
+                       base_symbol="lttng_channel_enable"
+                       offset=$(( 0x$channel_disable_addr - 0x$channel_enable_addr ))
+               else
+                       base_symbol="lttng_channel_disable"
+                       offset=$(( 0x$channel_enable_addr - 0x$channel_disable_addr ))
+               fi
+
+               offset_hex="0x$(printf '%x' $offset)"
+
+               test_success "--condition event-rule-matches probe by symbol with offset" \
+                       --condition event-rule-matches --domain=kernel --type=$type --location="${base_symbol}+${offset_hex}" --event-name=my_$base_symbol \
+                       --action notify
+
+               test_success "--condition event-rule-matches probe by address" \
+                       --condition event-rule-matches --domain=kernel --type=$type --location="0x${channel_enable_addr}" --event-name=my_channel_enable \
+                       --action notify
+       done
 }
 
-skip $ist_root "non-root user: skipping uprobe tests" 6 || {
-       test_success "--condition event-rule-matches uprobe" \
-               --condition event-rule-matches --domain=kernel --userspace-probe=${uprobe_elf_binary}:test_function --event-name=ma-probe \
-               --action notify
+skip $ist_root "non-root user: skipping uprobe tests" 12 || {
+       for type in uprobe userspace-probe; do
+               test_success "--condition event-rule-matches uprobe" \
+                       --condition event-rule-matches --domain=kernel --type=$type --location=${uprobe_elf_binary}:test_function --event-name=ma-probe \
+                       --action notify
 
-       test_success "--condition event-rule-matches uprobe with elf prefix" \
-               --condition event-rule-matches --domain=kernel --userspace-probe=elf:${uprobe_elf_binary}:test_function --event-name=ma-probe-2 \
-               --action notify
+               test_success "--condition event-rule-matches uprobe with elf prefix" \
+                       --condition event-rule-matches --domain=kernel --type=$type --location=elf:${uprobe_elf_binary}:test_function --event-name=ma-probe-2 \
+                       --action notify
+       done
 }
 
 skip $ist_root "non-root user: skipping syscall tests" 9 || {
        test_success "--condition event-rule-matches one syscall" \
-               --condition event-rule-matches --domain=kernel --syscall --name=open \
+               --condition event-rule-matches --domain=kernel --type=syscall --name=open \
                --action notify
 
        test_success "--condition event-rule-matches all syscalls" \
-               --condition event-rule-matches --domain=kernel --syscall \
+               --condition event-rule-matches --domain=kernel --type=syscall \
                --action notify
 
        test_success "--condition event-rule-matches one syscall with filter" \
-               --condition event-rule-matches --domain=kernel --syscall --filter 'a > 2' --name=open \
+               --condition event-rule-matches --domain=kernel --type=syscall --filter 'a > 2' --name=open \
                --action notify
 }
 
@@ -313,21 +317,29 @@ test_failure "two different --domain" \
        "Error: More than one \`--domain\` was specified." \
        --condition event-rule-matches --domain=user --domain=kernel
 
-test_failure "--condition event-rule-matches: --name with --probe" \
-       "Error: Can't use --name with probe event rules." \
-       --condition event-rule-matches --probe=do_sys_open --name='hello'
+for type in kprobe kernel-probe; do
+       test_failure "--condition event-rule-matches: --name with --type=$type" \
+               "Error: Can't use --name with probe event rules." \
+               --condition event-rule-matches --type=$type --location=do_sys_open --name='hello'
+done
+
+test_failure "--condition event-rule-matches: --location with tracepoint event rule" \
+       "Error: Can't use --location with tracepoint event rules." \
+       --condition event-rule-matches --domain=user --location='hello'
 
-test_failure "--condition event-rule-matches: --event-name with tracepoint" \
+test_failure "--condition event-rule-matches: --event-name with tracepoint event rule" \
        "Error: Can't use --event-name with tracepoint event rules." \
        --condition event-rule-matches --domain=user --event-name='hello'
 
-test_failure "--condition event-rule-matches: extra argument with --userspace-probe" \
-       "Error: Unexpected argument 'hello'" \
-       --condition event-rule-matches --domain=kernel --userspace-probe=${uprobe_elf_binary}:test_failure hello
+for type in uprobe userspace-probe; do
+       test_failure "--condition event-rule-matches: extra argument with --type=$type" \
+               "Error: Unexpected argument 'hello'" \
+               --condition event-rule-matches --domain=kernel --type=$type --location=${uprobe_elf_binary}:test_failure hello
+done
 
-test_failure "--condition event-rule-matches: extra argument with --syscall" \
+test_failure "--condition event-rule-matches: extra argument with --type=syscall" \
        "Error: Unexpected argument 'open'" \
-       --condition event-rule-matches --domain=kernel --syscall open
+       --condition event-rule-matches --domain=kernel --type=syscall open
 
 test_failure "--condition event-rule-matches --capture: missing argument (end of arg list)" \
        'Error: While parsing argument #2 (`--capture`): Missing required argument for option `--capture`' \

diff --git a/tests/regression/tools/trigger/test_list_triggers_cli b/tests/regression/tools/trigger/test_list_triggers_cli
index af0885d8555f2389ef37d04de10e8f0cb65cd8d2..30b636969c6e2135a683be02852932868e101667 100755 (executable)
--- a/tests/regression/tools/trigger/test_list_triggers_cli
+++ b/tests/regression/tools/trigger/test_list_triggers_cli
@@ -222,9 +222,9 @@ test_on_event_probe ()
 
        offset_hex="0x$(printf '%x' $offset)"
 
-       lttng_add_trigger_ok "T0" --condition event-rule-matches --domain=kernel --probe=lttng_channel_enable --event-name=my_channel_enable --action notify
-       lttng_add_trigger_ok "T1" --condition event-rule-matches --domain=kernel --probe="${base_symbol}+${offset_hex}" --event-name=my_channel_enable --action notify
-       lttng_add_trigger_ok "T2" --condition event-rule-matches --domain=kernel --probe="0x${channel_enable_addr}" --event-name=my_channel_enable --action notify
+       lttng_add_trigger_ok "T0" --condition event-rule-matches --domain=kernel --type=kprobe --location=lttng_channel_enable --event-name=my_channel_enable --action notify
+       lttng_add_trigger_ok "T1" --condition event-rule-matches --domain=kernel --type=kprobe --location="${base_symbol}+${offset_hex}" --event-name=my_channel_enable --action notify
+       lttng_add_trigger_ok "T2" --condition event-rule-matches --domain=kernel --type=kprobe --location="0x${channel_enable_addr}" --event-name=my_channel_enable --action notify
 
        cat > "${tmp_expected_stdout}" <<- EOF
        - name: T0
@@ -266,7 +266,7 @@ test_on_event_userspace_probe_elf ()
 
        diag "Listing on-event userspace-probe elf"
 
-       lttng_add_trigger_ok "T0" --condition event-rule-matches --domain=kernel --userspace-probe=${uprobe_elf_binary}:test_function --event-name=ma-probe-elf --action notify
+       lttng_add_trigger_ok "T0" --condition event-rule-matches --domain=kernel --type=uprobe --location=${uprobe_elf_binary}:test_function --event-name=ma-probe-elf --action notify
 
        cat > "${tmp_expected_stdout}" <<- EOF
        - name: T0
@@ -313,8 +313,8 @@ test_on_event_syscall ()
 {
        diag "Listing on-event syscall"
 
-       lttng_add_trigger_ok "T0" --condition event-rule-matches --domain=kernel --syscall --name=open --action notify
-       lttng_add_trigger_ok "T1" --condition event-rule-matches --domain=kernel --syscall --name=ptrace --filter 'a > 2' --action notify
+       lttng_add_trigger_ok "T0" --condition event-rule-matches --domain=kernel --type=syscall --name=open --action notify
+       lttng_add_trigger_ok "T1" --condition event-rule-matches --domain=kernel --type=syscall --name=ptrace --filter 'a > 2' --action notify
 
        cat > "${tmp_expected_stdout}" <<- EOF
        - name: T0