From 491c35cc5b4db44f74efa3294777ec7d326c8ae7 Mon Sep 17 00:00:00 2001 From: Philippe Proulx Date: Tue, 4 Jun 2019 16:58:26 -0400 Subject: [PATCH] sink.ctf.fs: add more comments in code where it's not straightforward This patch adds a few comments in the code of `sink.ctf.fs` to help future contributors without deep knowledge of CTF. Signed-off-by: Philippe Proulx Change-Id: Ibaa9ae27adb42f76329d0eab5a321368f452fc56 Reviewed-on: https://review.lttng.org/c/babeltrace/+/1376 Tested-by: jenkins Reviewed-by: Simon Marchi --- plugins/ctf/fs-sink/fs-sink-stream.h | 91 +++++++++++++++++- plugins/ctf/fs-sink/fs-sink.c | 136 +++++++++++++++++++++++---- plugins/ctf/fs-sink/fs-sink.h | 15 +++ 3 files changed, 221 insertions(+), 21 deletions(-) diff --git a/plugins/ctf/fs-sink/fs-sink-stream.h b/plugins/ctf/fs-sink/fs-sink-stream.h index 9f48ce85..ad6fdcfd 100644 --- a/plugins/ctf/fs-sink/fs-sink-stream.h +++ b/plugins/ctf/fs-sink/fs-sink-stream.h @@ -46,39 +46,126 @@ struct fs_sink_stream { struct fs_sink_ctf_stream_class *sc; + /* Current packet's state */ struct { + /* + * True if we're, for this stream, within an opened + * packet (got a packet beginning message, but no + * packet end message yet). + */ bool is_open; + + /* + * Current beginning default clock snapshot for the + * current packet (`UINT64_C(-1)` if not set). + */ uint64_t beginning_cs; + + /* + * Current end default clock snapshot for the current + * packet (`UINT64_C(-1)` if not set). + */ uint64_t end_cs; + + /* + * Current packet's content size (bits) for the current + * packet. + */ uint64_t content_size; + + /* + * Current packet's total size (bits) for the current + * packet. + */ uint64_t total_size; + + /* + * Discarded events (free running) counter for the + * current packet. + */ uint64_t discarded_events_counter; + + /* Sequence number (free running) of the current packet */ uint64_t seq_num; + + /* + * Offset of the packet context structure within the + * current packet (bits). + */ uint64_t context_offset_bits; /* Owned by this */ const bt_packet *packet; } packet_state; + /* Previous packet's state */ struct { + /* End default clock snapshot (`UINT64_C(-1)` if not set) */ uint64_t end_cs; + + /* Discarded events (free running) counter */ uint64_t discarded_events_counter; + + /* Sequence number (free running) */ uint64_t seq_num; } prev_packet_state; + /* State to handle discarded events */ struct { + /* + * True if we're in the time range given by a previously + * received discarded events message. In this case, + * `beginning_cs` and `end_cs` below contain the + * beginning and end clock snapshots for this range. + * + * This is used to validate that, when receiving a + * packet end message, the current discarded events time + * range matches what's expected for CTF 1.8, that is: + * + * * Its beginning time is the previous packet's end + * time (or the current packet's beginning time if + * this is the first packet). + * + * * Its end time is the current packet's end time. + */ bool in_range; + + /* + * Beginning and end times of the time range given by a + * previously received discarded events message. + */ uint64_t beginning_cs; uint64_t end_cs; } discarded_events_state; + /* State to handle discarded packets */ struct { + /* + * True if we're in the time range given by a previously + * received discarded packets message. In this case, + * `beginning_cs` and `end_cs` below contain the + * beginning and end clock snapshots for this range. + * + * This is used to validate that, when receiving a + * packet beginning message, the current discarded + * packets time range matches what's expected for CTF + * 1.8, that is: + * + * * Its beginning time is the previous packet's end + * time. + * + * * Its end time is the current packet's beginning + * time. + */ bool in_range; + + /* + * Beginning and end times of the time range given by a + * previously received discarded packets message. + */ uint64_t beginning_cs; uint64_t end_cs; } discarded_packets_state; - - bool in_discarded_events_range; }; BT_HIDDEN diff --git a/plugins/ctf/fs-sink/fs-sink.c b/plugins/ctf/fs-sink/fs-sink.c index 5f04496f..9cc43883 100644 --- a/plugins/ctf/fs-sink/fs-sink.c +++ b/plugins/ctf/fs-sink/fs-sink.c @@ -326,17 +326,38 @@ bt_self_component_status handle_packet_beginning_msg( BT_ASSERT(cs); } + /* + * If we previously received a discarded events message with + * a time range, make sure that its beginning time matches what's + * expected for CTF 1.8, that is: + * + * * Its beginning time is the previous packet's end + * time (or the current packet's beginning time if + * this is the first packet). + * + * We check this here instead of in handle_packet_end_msg() + * because we want to catch any incompatible message as early as + * possible to report the error. + * + * Validation of the discarded events message's end time is + * performed in handle_packet_end_msg(). + */ if (stream->discarded_events_state.in_range) { uint64_t expected_cs; + /* + * `stream->discarded_events_state.in_range` is only set + * when the stream class's discarded events have a time + * range. + * + * It is required that the packet beginning and end + * messages for this stream class have times when + * discarded events have a time range. + */ BT_ASSERT(stream->sc->discarded_events_has_ts); BT_ASSERT(stream->sc->packets_have_ts_begin); BT_ASSERT(stream->sc->packets_have_ts_end); - /* - * Make sure that the current discarded events range's - * beginning time matches what's expected for CTF 1.8. - */ if (stream->prev_packet_state.end_cs == UINT64_C(-1)) { /* We're opening the first packet */ expected_cs = bt_clock_snapshot_get_value(cs); @@ -364,17 +385,36 @@ bt_self_component_status handle_packet_beginning_msg( } } + /* + * If we previously received a discarded packets message with a + * time range, make sure that its beginning and end times match + * what's expected for CTF 1.8, that is: + * + * * Its beginning time is the previous packet's end time. + * + * * Its end time is the current packet's beginning time. + */ if (stream->discarded_packets_state.in_range) { uint64_t expected_end_cs; + /* + * `stream->discarded_packets_state.in_range` is only + * set when the stream class's discarded packets have a + * time range. + * + * It is required that the packet beginning and end + * messages for this stream class have times when + * discarded packets have a time range. + */ BT_ASSERT(stream->sc->discarded_packets_has_ts); BT_ASSERT(stream->sc->packets_have_ts_begin); BT_ASSERT(stream->sc->packets_have_ts_end); /* - * Make sure that the current discarded packets range's - * beginning and end times match what's expected for CTF - * 1.8. + * It is not supported to have a discarded packets + * message _before_ the first packet: we cannot validate + * that its beginning time is compatible with CTF 1.8 in + * this case. */ if (stream->prev_packet_state.end_cs == UINT64_C(-1)) { BT_LOGE("Incompatible discarded packets message " @@ -431,9 +471,13 @@ bt_self_component_status handle_packet_beginning_msg( } } - if (stream->sc->discarded_packets_has_ts) { - stream->discarded_packets_state.in_range = false; - } + /* + * We're not in a discarded packets time range anymore since we + * require that the discarded packets time ranges go from one + * packet's end time to the next packet's beginning time, and + * we're handling a packet beginning message here. + */ + stream->discarded_packets_state.in_range = false; ret = fs_sink_stream_open_packet(stream, cs, ir_packet); if (ret) { @@ -469,17 +513,32 @@ bt_self_component_status handle_packet_end_msg( BT_ASSERT(cs); } + /* + * If we previously received a discarded events message with + * a time range, make sure that its end time matches what's + * expected for CTF 1.8, that is: + * + * * Its end time is the current packet's end time. + * + * Validation of the discarded events message's beginning time + * is performed in handle_packet_beginning_msg(). + */ if (stream->discarded_events_state.in_range) { uint64_t expected_cs; + /* + * `stream->discarded_events_state.in_range` is only set + * when the stream class's discarded events have a time + * range. + * + * It is required that the packet beginning and end + * messages for this stream class have times when + * discarded events have a time range. + */ BT_ASSERT(stream->sc->discarded_events_has_ts); BT_ASSERT(stream->sc->packets_have_ts_begin); BT_ASSERT(stream->sc->packets_have_ts_end); - /* - * Make sure that the current discarded events range's - * end time matches what's expected for CTF 1.8. - */ expected_cs = bt_clock_snapshot_get_value(cs); if (stream->discarded_events_state.end_cs != expected_cs) { @@ -507,9 +566,13 @@ bt_self_component_status handle_packet_end_msg( goto end; } - if (stream->sc->discarded_events_has_ts) { - stream->discarded_events_state.in_range = false; - } + /* + * We're not in a discarded events time range anymore since we + * require that the discarded events time ranges go from one + * packet's end time to the next packet's end time, and we're + * handling a packet end message here. + */ + stream->discarded_events_state.in_range = false; end: return status; @@ -660,6 +723,14 @@ bt_self_component_status handle_discarded_events_msg( goto end; } + /* + * If we're currently in an opened packet (got a packet + * beginning message, but no packet end message yet), we do not + * support having a discarded events message with a time range + * because we require that the discarded events message's time + * range go from a packet's end time to the next packet's end + * time. + */ if (stream->packet_state.is_open && stream->sc->discarded_events_has_ts) { BT_LOGE("Unsupported discarded events message with " @@ -676,11 +747,18 @@ bt_self_component_status handle_discarded_events_msg( } if (stream->sc->discarded_events_has_ts) { + /* + * Make the stream's state be in the time range of a + * discarded events message since we have the message's + * time range (`stream->sc->discarded_events_has_ts`). + */ stream->discarded_events_state.in_range = true; /* * The clock snapshot values will be validated when - * handling the next "packet beginning" message. + * handling the next packet beginning and end messages + * (next calls to handle_packet_beginning_msg() and + * handle_packet_end_msg()). */ cs = bt_message_discarded_events_borrow_beginning_default_clock_snapshot_const( msg); @@ -695,6 +773,11 @@ bt_self_component_status handle_discarded_events_msg( avail = bt_message_discarded_events_get_count(msg, &count); if (avail != BT_PROPERTY_AVAILABILITY_AVAILABLE) { + /* + * There's no specific count of discarded events: set it + * to 1 so that we know that we at least discarded + * something. + */ count = 1; } @@ -747,14 +830,24 @@ bt_self_component_status handle_discarded_packets_msg( goto end; } + /* + * Discarded packets messages are guaranteed to occur between + * packets. + */ BT_ASSERT(!stream->packet_state.is_open); if (stream->sc->discarded_packets_has_ts) { + /* + * Make the stream's state be in the time range of a + * discarded packets message since we have the message's + * time range (`stream->sc->discarded_packets_has_ts`). + */ stream->discarded_packets_state.in_range = true; /* * The clock snapshot values will be validated when - * handling the next "packet beginning" message. + * handling the next packet beginning message (next call + * to handle_packet_beginning_msg()). */ cs = bt_message_discarded_packets_borrow_beginning_default_clock_snapshot_const( msg); @@ -770,6 +863,11 @@ bt_self_component_status handle_discarded_packets_msg( avail = bt_message_discarded_packets_get_count(msg, &count); if (avail != BT_PROPERTY_AVAILABILITY_AVAILABLE) { + /* + * There's no specific count of discarded packets: set + * it to 1 so that we know that we at least discarded + * something. + */ count = 1; } diff --git a/plugins/ctf/fs-sink/fs-sink.h b/plugins/ctf/fs-sink/fs-sink.h index a41052c2..c9b94ae3 100644 --- a/plugins/ctf/fs-sink/fs-sink.h +++ b/plugins/ctf/fs-sink/fs-sink.h @@ -37,9 +37,24 @@ struct fs_sink_comp { /* Base output directory path */ GString *output_dir_path; + /* + * True if the component assumes that it will only write a + * single CTF trace (which can contain one or more data + * streams). This makes the component write the stream files + * directly in the output directory (`output_dir_path` above). + */ bool assume_single_trace; + + /* True to completely ignore discarded events messages */ bool ignore_discarded_events; + + /* True to completely ignore discarded packets messages */ bool ignore_discarded_packets; + + /* + * True to make the component quiet (nothing printed to the + * standard output). + */ bool quiet; /* -- 2.34.1