From bacc39bbfbccc984d2b558b41cf8ab42a8592d09 Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 6 Jan 2022 13:24:59 -0500 Subject: [PATCH] Fix: lttng-sessiond: output stream metadata before events MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Jérémie Galarneau --- src/bin/lttng-sessiond/cmd.cpp | 5 +++- src/bin/lttng-sessiond/ust-metadata.cpp | 36 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/bin/lttng-sessiond/cmd.cpp b/src/bin/lttng-sessiond/cmd.cpp index 49fa0c686..53a175372 100644 --- 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(®istry->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 d08cb40ae..566799efd 100644 --- a/src/bin/lttng-sessiond/ust-metadata.cpp +++ b/src/bin/lttng-sessiond/ust-metadata.cpp @@ -15,6 +15,7 @@ #include #include #include +#include #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 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; } -- 2.34.1