Fix: lttng-sessiond: output stream metadata before events

When trying the `doc/examples/demo` example from the lttng-ust
repository, the resulting trace's metadata lists some events before the
corresponding stream declaration. Here's an excerpt:

    event {
            name = "lttng_ust_statedump:end";
        id = 5;
        stream_id = 0;
        loglevel = 13;
        fields := struct {
            };
    };

    stream {
            id = 0;
            event.header := struct event_header_large;
            packet.context := struct packet_context;
    };

I don't know if this is allowed in CTF 1, but it won't be in CTF 2 (an
event record class fragment must come after its parent data stream class
fragment). In any case, I think it makes more sense to have the stream
first.

What I can see is that the ust_metadata_event_statedump function (which
emits the `event` declarations) is called for the statedump events
before the ust_metadata_channel_statedump function (which emits the
`stream` declaration) is called. A simple fix, as implemented in this
patch, is to delay emitting the event declarations until the stream
declaration has been emitted. To do so, return early in
ust_metadata_event_statedump if the `chan->metadata_dumped` flag is not
set. Then, when emitting the stream declaration, in
ust_metadata_event_statedump, emit any existing event, which have
presumably been skipped before hand.

It's possible that ust_metadata_event_statedump getting called before
ust_metadata_channel_statedump is a symptom of some more fundamental
problem over which this patch only papers over, but I don't know enough
about this to be able to tell.

I couldn't think of an appropriate test to write for this. However, once
we generate CTF2, such a bug would likely be caught by trace readers
rejecting the invlid metadata. So if we were to re-introduce this bug,
we would notice.

Change-Id: I6e3158c801fcc01b318618890704d19b3230e7a5
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-sessiond/cmd.cpp b/src/bin/lttng-sessiond/cmd.cpp
index 49fa0c6864401f9aa6a95872c7ab9b4bf5e9a307..53a175372dadc5099b97d98d5795cf51a03ae600 100644 (file)
--- a/src/bin/lttng-sessiond/cmd.cpp
+++ b/src/bin/lttng-sessiond/cmd.cpp
@@ -4359,6 +4359,8 @@ int ust_regenerate_metadata(struct ltt_ust_session *usess)
                        struct ust_registry_event *event;
                        struct lttng_ht_iter iter_event;
 
+                       chan->metadata_dumped = 0;
+
                        ret = ust_metadata_channel_statedump(registry, chan);
                        if (ret) {
                                pthread_mutex_unlock(&registry->lock);
@@ -4367,7 +4369,8 @@ int ust_regenerate_metadata(struct ltt_ust_session *usess)
                                goto end;
                        }
                        cds_lfht_for_each_entry(chan->events->ht, &iter_event.iter,
-                                       event, node.node) {
+                                               event, node.node) {
+                               event->metadata_dumped = 0;
                                ret = ust_metadata_event_statedump(registry,
                                                chan, event);
                                if (ret) {

diff --git a/src/bin/lttng-sessiond/ust-metadata.cpp b/src/bin/lttng-sessiond/ust-metadata.cpp
index d08cb40ae7f10ade8a8add223997a2e7aa884bed..566799efd08b2016e7ce24edeeadb3cd70937d04 100644 (file)
--- a/src/bin/lttng-sessiond/ust-metadata.cpp
+++ b/src/bin/lttng-sessiond/ust-metadata.cpp
@@ -15,6 +15,7 @@
 #include <inttypes.h>
 #include <common/common.hpp>
 #include <common/time.hpp>
+#include <vector>
 
 #include "ust-registry.hpp"
 #include "ust-clock.hpp"
@@ -841,6 +842,16 @@ int ust_metadata_event_statedump(struct ust_registry_session *session,
        if (chan->chan_id == -1U)
                return 0;
 
+       /*
+        * We don't want to output an event's metadata before its parent
+        * stream's metadata.  If the stream's metadata hasn't been output yet,
+        * skip this event.  Its metadata will be output when we output the
+        * stream's metadata.
+        */
+       if (!chan->metadata_dumped || event->metadata_dumped) {
+               return 0;
+       }
+
        ret = lttng_metadata_printf(session,
                "event {\n"
                "       name = \"%s\";\n"
@@ -951,6 +962,31 @@ int ust_metadata_channel_statedump(struct ust_registry_session *session,
        /* Flag success of metadata dump. */
        chan->metadata_dumped = 1;
 
+       /*
+        * Output the metadata of any existing event.
+        *
+        * Sort the events by id.  This is not necessary, but it's nice to have
+        * a more predictable order in the metadata file.
+        */
+       std::vector<ust_registry_event *> events;
+       {
+               cds_lfht_iter event_iter;
+               ust_registry_event *event;
+               cds_lfht_for_each_entry(chan->events->ht, &event_iter, event,
+                               node.node) {
+                       events.push_back(event);
+               }
+       }
+
+       std::sort(events.begin(), events.end(),
+               [] (ust_registry_event *a, ust_registry_event *b) {
+                       return a->id < b->id;
+               });
+
+       for (ust_registry_event *event : events) {
+               ust_metadata_event_statedump(session, chan, event);
+       }
+
        return 0;
 }