From: Simon Marchi <simon.marchi@efficios.com>
Date: Thu, 6 Jan 2022 18:24:59 +0000 (-0500)
Subject: Fix: lttng-sessiond: output stream metadata before events
X-Git-Url: http://drtracing.org/?a=commitdiff_plain;h=bacc39bbfbccc984d2b558b41cf8ab42a8592d09;p=deliverable%2Flttng-tools.git

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 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(&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 d08cb40ae..566799efd 100644
--- 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;
 }