From 3ff5c5db220d92baf64280ba54713fcafe76142e Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 11 Dec 2019 13:19:13 -0500 Subject: [PATCH] common: index and trace-chunk file creation/open API change MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit An API change to the common libraries is required for the following functions to accomodate the clear feature: - lttng_trace_chunk_open_file(), - lttng_index_file_create_from_trace_chunk(), - lttng_index_file_create_from_trace_chunk_read_only(). An "expect_no_file" boolean is introduced, allowing specific invocation to consider a ENOENT errno as an expected condition (and not an error). This is the case for instance for relayd live viewer_stream_rotate(), which can fail to open files in the new chunk after a clear with per-pid buffers. Same goes for viewer_get_next_index() and try_open_index(). Change the lttng_index_file_create_from_trace_chunk_read_only and lttng_index_file_create_from_trace_chunk to allow them to return a chunk status, which allows the callers to distinguish between errors and a "no file" status, and deal with the situation accordingly. Signed-off-by: Mathieu Desnoyers Change-Id: I3c43edca01629488c4e777078db7ffd2e88dc7ea Signed-off-by: Jérémie Galarneau --- src/bin/lttng-relayd/live.c | 28 ++++++++++++++++++----- src/bin/lttng-relayd/stream.c | 9 ++++---- src/bin/lttng-relayd/viewer-stream.c | 23 ++++++++++++------- src/common/consumer/consumer-stream.c | 8 +++---- src/common/index/index.c | 33 ++++++++++++++++++--------- src/common/index/index.h | 11 +++++---- src/common/trace-chunk.c | 10 +++++--- src/common/trace-chunk.h | 3 ++- 8 files changed, 84 insertions(+), 41 deletions(-) diff --git a/src/bin/lttng-relayd/live.c b/src/bin/lttng-relayd/live.c index 1b25671d5..250d85c6f 100644 --- a/src/bin/lttng-relayd/live.c +++ b/src/bin/lttng-relayd/live.c @@ -1222,6 +1222,7 @@ static int try_open_index(struct relay_viewer_stream *vstream, int ret = 0; const uint32_t connection_major = rstream->trace->session->major; const uint32_t connection_minor = rstream->trace->session->minor; + enum lttng_trace_chunk_status chunk_status; if (vstream->index_file) { goto end; @@ -1234,14 +1235,19 @@ static int try_open_index(struct relay_viewer_stream *vstream, ret = -ENOENT; goto end; } - vstream->index_file = lttng_index_file_create_from_trace_chunk_read_only( + chunk_status = lttng_index_file_create_from_trace_chunk_read_only( vstream->stream_file.trace_chunk, rstream->path_name, rstream->channel_name, rstream->tracefile_size, vstream->current_tracefile_id, lttng_to_index_major(connection_major, connection_minor), - lttng_to_index_minor(connection_major, connection_minor)); - if (!vstream->index_file) { - ret = -1; + lttng_to_index_minor(connection_major, connection_minor), + true, &vstream->index_file); + if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { + if (chunk_status == LTTNG_TRACE_CHUNK_STATUS_NO_FILE) { + ret = -ENOENT; + } else { + ret = -1; + } } end: @@ -1460,9 +1466,14 @@ int viewer_get_next_index(struct relay_connection *conn) goto error_put; } + /* + * It is possible the the file we are trying to open is + * missing if the stream has been closed (application exits with + * per-pid buffers) and a clear command has been performed. + */ status = lttng_trace_chunk_open_file( vstream->stream_file.trace_chunk, - file_path, O_RDONLY, 0, &fd); + file_path, O_RDONLY, 0, &fd, true); if (status != LTTNG_TRACE_CHUNK_STATUS_OK) { PERROR("Failed to open trace file for viewer stream"); goto error_put; @@ -1759,9 +1770,14 @@ int viewer_get_metadata(struct relay_connection *conn) goto error; } + /* + * It is possible the the metadata file we are trying to open is + * missing if the stream has been closed (application exits with + * per-pid buffers) and a clear command has been performed. + */ status = lttng_trace_chunk_open_file( vstream->stream_file.trace_chunk, - file_path, O_RDONLY, 0, &fd); + file_path, O_RDONLY, 0, &fd, true); if (status != LTTNG_TRACE_CHUNK_STATUS_OK) { PERROR("Failed to open metadata file for viewer stream"); goto error; diff --git a/src/bin/lttng-relayd/stream.c b/src/bin/lttng-relayd/stream.c index d34158bd2..501e6e90a 100644 --- a/src/bin/lttng-relayd/stream.c +++ b/src/bin/lttng-relayd/stream.c @@ -123,7 +123,7 @@ static int stream_create_data_output_file_from_trace_chunk( } status = lttng_trace_chunk_open_file( - trace_chunk, stream_path, flags, mode, &fd); + trace_chunk, stream_path, flags, mode, &fd, false); if (status != LTTNG_TRACE_CHUNK_STATUS_OK) { ERR("Failed to open stream file \"%s\"", stream->channel_name); ret = -1; @@ -445,13 +445,14 @@ static int create_index_file(struct relay_stream *stream, ret = -1; goto end; } - stream->index_file = lttng_index_file_create_from_trace_chunk( + status = lttng_index_file_create_from_trace_chunk( chunk, stream->path_name, stream->channel_name, stream->tracefile_size, stream->tracefile_current_index, lttng_to_index_major(major, minor), - lttng_to_index_minor(major, minor), true); - if (!stream->index_file) { + lttng_to_index_minor(major, minor), true, + &stream->index_file); + if (status != LTTNG_TRACE_CHUNK_STATUS_OK) { ret = -1; goto end; } diff --git a/src/bin/lttng-relayd/viewer-stream.c b/src/bin/lttng-relayd/viewer-stream.c index acd488b61..70654c797 100644 --- a/src/bin/lttng-relayd/viewer-stream.c +++ b/src/bin/lttng-relayd/viewer-stream.c @@ -129,8 +129,9 @@ struct relay_viewer_stream *viewer_stream_create(struct relay_stream *stream, } else { const uint32_t connection_major = stream->trace->session->major; const uint32_t connection_minor = stream->trace->session->minor; + enum lttng_trace_chunk_status chunk_status; - vstream->index_file = lttng_index_file_create_from_trace_chunk_read_only( + chunk_status = lttng_index_file_create_from_trace_chunk_read_only( vstream->stream_file.trace_chunk, stream->path_name, stream->channel_name, stream->tracefile_size, @@ -138,9 +139,14 @@ struct relay_viewer_stream *viewer_stream_create(struct relay_stream *stream, lttng_to_index_major(connection_major, connection_minor), lttng_to_index_minor(connection_major, - connection_minor)); - if (!vstream->index_file) { - goto error_unlock; + connection_minor), + true, &vstream->index_file); + if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { + if (chunk_status == LTTNG_TRACE_CHUNK_STATUS_NO_FILE) { + vstream->index_file = NULL; + } else { + goto error_unlock; + } } } @@ -268,6 +274,7 @@ int viewer_stream_rotate(struct relay_viewer_stream *vstream) const struct relay_stream *stream = vstream->stream; const uint32_t connection_major = stream->trace->session->major; const uint32_t connection_minor = stream->trace->session->minor; + enum lttng_trace_chunk_status chunk_status; /* Detect the last tracefile to open. */ if (stream->index_received_seqcount @@ -316,8 +323,7 @@ int viewer_stream_rotate(struct relay_viewer_stream *vstream) stream_fd_put(vstream->stream_file.fd); vstream->stream_file.fd = NULL; } - vstream->index_file = - lttng_index_file_create_from_trace_chunk_read_only( + chunk_status = lttng_index_file_create_from_trace_chunk_read_only( vstream->stream_file.trace_chunk, stream->path_name, stream->channel_name, @@ -326,8 +332,9 @@ int viewer_stream_rotate(struct relay_viewer_stream *vstream) lttng_to_index_major(connection_major, connection_minor), lttng_to_index_minor(connection_major, - connection_minor)); - if (!vstream->index_file) { + connection_minor), + true, &vstream->index_file); + if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { ret = -1; goto end; } else { diff --git a/src/common/consumer/consumer-stream.c b/src/common/consumer/consumer-stream.c index cdda9412e..daed3866f 100644 --- a/src/common/consumer/consumer-stream.c +++ b/src/common/consumer/consumer-stream.c @@ -594,7 +594,7 @@ int consumer_stream_create_output_files(struct lttng_consumer_stream *stream, DBG("Opening stream output file \"%s\"", stream_path); chunk_status = lttng_trace_chunk_open_file(stream->trace_chunk, stream_path, - flags, mode, &stream->out_fd); + flags, mode, &stream->out_fd, false); if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { ERR("Failed to open stream file \"%s\"", stream->name); ret = -1; @@ -605,15 +605,15 @@ int consumer_stream_create_output_files(struct lttng_consumer_stream *stream, if (stream->index_file) { lttng_index_file_put(stream->index_file); } - stream->index_file = lttng_index_file_create_from_trace_chunk( + chunk_status = lttng_index_file_create_from_trace_chunk( stream->trace_chunk, stream->chan->pathname, stream->name, stream->chan->tracefile_size, stream->tracefile_count_current, CTF_INDEX_MAJOR, CTF_INDEX_MINOR, - false); - if (!stream->index_file) { + false, &stream->index_file); + if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { ret = -1; goto end; } diff --git a/src/common/index/index.c b/src/common/index/index.c index 540294a5e..9f7e5f67a 100644 --- a/src/common/index/index.c +++ b/src/common/index/index.c @@ -34,13 +34,13 @@ #define WRITE_FILE_FLAGS (O_WRONLY | O_CREAT | O_TRUNC) #define READ_ONLY_FILE_FLAGS O_RDONLY -static struct lttng_index_file *_lttng_index_file_create_from_trace_chunk( +static enum lttng_trace_chunk_status _lttng_index_file_create_from_trace_chunk( struct lttng_trace_chunk *chunk, const char *channel_path, const char *stream_name, uint64_t stream_file_size, uint64_t stream_file_index, uint32_t index_major, uint32_t index_minor, bool unlink_existing_file, - int flags) + int flags, bool expect_no_file, struct lttng_index_file **file) { struct lttng_index_file *index_file; enum lttng_trace_chunk_status chunk_status; @@ -58,6 +58,7 @@ static struct lttng_index_file *_lttng_index_file_create_from_trace_chunk( index_file = zmalloc(sizeof(*index_file)); if (!index_file) { PERROR("Failed to allocate lttng_index_file"); + chunk_status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto error; } @@ -71,6 +72,7 @@ static struct lttng_index_file *_lttng_index_file_create_from_trace_chunk( "%s%s" DEFAULT_INDEX_DIR, channel_path, separator); if (ret < 0 || ret >= sizeof(index_directory_path)) { ERR("Failed to format index directory path"); + chunk_status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto error; } @@ -79,6 +81,7 @@ static struct lttng_index_file *_lttng_index_file_create_from_trace_chunk( DEFAULT_INDEX_FILE_SUFFIX, index_file_path, sizeof(index_file_path)); if (ret) { + chunk_status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto error; } @@ -101,7 +104,7 @@ static struct lttng_index_file *_lttng_index_file_create_from_trace_chunk( } chunk_status = lttng_trace_chunk_open_file(chunk, index_file_path, - flags, mode, &fd); + flags, mode, &fd, expect_no_file); if (chunk_status != LTTNG_TRACE_CHUNK_STATUS_OK) { goto error; } @@ -111,6 +114,7 @@ static struct lttng_index_file *_lttng_index_file_create_from_trace_chunk( size_ret = lttng_write(fd, &hdr, sizeof(hdr)); if (size_ret < sizeof(hdr)) { PERROR("Failed to write index header"); + chunk_status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto error; } index_file->element_len = ctf_packet_index_len(index_major, index_minor); @@ -120,25 +124,30 @@ static struct lttng_index_file *_lttng_index_file_create_from_trace_chunk( size_ret = lttng_read(fd, &hdr, sizeof(hdr)); if (size_ret < 0) { PERROR("Failed to read index header"); + chunk_status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto error; } if (be32toh(hdr.magic) != CTF_INDEX_MAGIC) { ERR("Invalid header magic"); + chunk_status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto error; } if (index_major != be32toh(hdr.index_major)) { ERR("Index major number mismatch: %u, expect %u", be32toh(hdr.index_major), index_major); + chunk_status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto error; } if (index_minor != be32toh(hdr.index_minor)) { ERR("Index minor number mismatch: %u, expect %u", be32toh(hdr.index_minor), index_minor); + chunk_status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto error; } element_len = be32toh(hdr.packet_index_len); if (element_len > sizeof(struct ctf_packet_index)) { ERR("Index element length too long"); + chunk_status = LTTNG_TRACE_CHUNK_STATUS_ERROR; goto error; } index_file->element_len = element_len; @@ -148,7 +157,8 @@ static struct lttng_index_file *_lttng_index_file_create_from_trace_chunk( index_file->minor = index_minor; urcu_ref_init(&index_file->ref); - return index_file; + *file = index_file; + return LTTNG_TRACE_CHUNK_STATUS_OK; error: if (fd >= 0) { @@ -159,32 +169,33 @@ error: } lttng_trace_chunk_put(chunk); free(index_file); - return NULL; + return chunk_status; } -struct lttng_index_file *lttng_index_file_create_from_trace_chunk( +enum lttng_trace_chunk_status lttng_index_file_create_from_trace_chunk( struct lttng_trace_chunk *chunk, const char *channel_path, const char *stream_name, uint64_t stream_file_size, uint64_t stream_file_index, uint32_t index_major, uint32_t index_minor, - bool unlink_existing_file) + bool unlink_existing_file, struct lttng_index_file **file) { return _lttng_index_file_create_from_trace_chunk(chunk, channel_path, stream_name, stream_file_size, stream_file_index, index_major, index_minor, unlink_existing_file, - WRITE_FILE_FLAGS); + WRITE_FILE_FLAGS, false, file); } -struct lttng_index_file *lttng_index_file_create_from_trace_chunk_read_only( +enum lttng_trace_chunk_status lttng_index_file_create_from_trace_chunk_read_only( struct lttng_trace_chunk *chunk, const char *channel_path, const char *stream_name, uint64_t stream_file_size, uint64_t stream_file_index, - uint32_t index_major, uint32_t index_minor) + uint32_t index_major, uint32_t index_minor, + bool expect_no_file, struct lttng_index_file **file) { return _lttng_index_file_create_from_trace_chunk(chunk, channel_path, stream_name, stream_file_size, stream_file_index, index_major, index_minor, false, - READ_ONLY_FILE_FLAGS); + READ_ONLY_FILE_FLAGS, expect_no_file, file); } /* diff --git a/src/common/index/index.h b/src/common/index/index.h index 3dbe458f3..f42e31e2d 100644 --- a/src/common/index/index.h +++ b/src/common/index/index.h @@ -39,17 +39,20 @@ struct lttng_index_file { * create and open have refcount of 1. Use put to decrement the * refcount. Destroys when reaching 0. Use "get" to increment refcount. */ -struct lttng_index_file *lttng_index_file_create_from_trace_chunk( +enum lttng_trace_chunk_status lttng_index_file_create_from_trace_chunk( struct lttng_trace_chunk *chunk, const char *channel_path, const char *stream_name, uint64_t stream_file_size, uint64_t stream_count, uint32_t index_major, uint32_t index_minor, - bool unlink_existing_file); -struct lttng_index_file *lttng_index_file_create_from_trace_chunk_read_only( + bool unlink_existing_file, struct lttng_index_file **file); + +enum lttng_trace_chunk_status lttng_index_file_create_from_trace_chunk_read_only( struct lttng_trace_chunk *chunk, const char *channel_path, const char *stream_name, uint64_t stream_file_size, uint64_t stream_file_index, - uint32_t index_major, uint32_t index_minor); + uint32_t index_major, uint32_t index_minor, + bool expect_no_file, struct lttng_index_file **file); + int lttng_index_file_write(const struct lttng_index_file *index_file, const struct ctf_packet_index *element); int lttng_index_file_read(const struct lttng_index_file *index_file, diff --git a/src/common/trace-chunk.c b/src/common/trace-chunk.c index 6181520fa..aadb40f6f 100644 --- a/src/common/trace-chunk.c +++ b/src/common/trace-chunk.c @@ -816,7 +816,7 @@ end: LTTNG_HIDDEN enum lttng_trace_chunk_status lttng_trace_chunk_open_file( struct lttng_trace_chunk *chunk, const char *file_path, - int flags, mode_t mode, int *out_fd) + int flags, mode_t mode, int *out_fd, bool expect_no_file) { int ret; enum lttng_trace_chunk_status status = LTTNG_TRACE_CHUNK_STATUS_OK; @@ -844,9 +844,13 @@ enum lttng_trace_chunk_status lttng_trace_chunk_open_file( chunk->credentials.value.use_current_user ? NULL : &chunk->credentials.value.user); if (ret < 0) { - PERROR("Failed to open file relative to trace chunk file_path = \"%s\", flags = %d, mode = %d", + if (errno == ENOENT && expect_no_file) { + status = LTTNG_TRACE_CHUNK_STATUS_NO_FILE; + } else { + PERROR("Failed to open file relative to trace chunk file_path = \"%s\", flags = %d, mode = %d", file_path, flags, (int) mode); - status = LTTNG_TRACE_CHUNK_STATUS_ERROR; + status = LTTNG_TRACE_CHUNK_STATUS_ERROR; + } goto end; } *out_fd = ret; diff --git a/src/common/trace-chunk.h b/src/common/trace-chunk.h index fcacb01ff..b90daf8ae 100644 --- a/src/common/trace-chunk.h +++ b/src/common/trace-chunk.h @@ -68,6 +68,7 @@ enum lttng_trace_chunk_status { LTTNG_TRACE_CHUNK_STATUS_INVALID_ARGUMENT, LTTNG_TRACE_CHUNK_STATUS_INVALID_OPERATION, LTTNG_TRACE_CHUNK_STATUS_ERROR, + LTTNG_TRACE_CHUNK_STATUS_NO_FILE, }; enum lttng_trace_chunk_command_type { @@ -154,7 +155,7 @@ enum lttng_trace_chunk_status lttng_trace_chunk_create_subdirectory( LTTNG_HIDDEN enum lttng_trace_chunk_status lttng_trace_chunk_open_file( struct lttng_trace_chunk *chunk, const char *filename, - int flags, mode_t mode, int *out_fd); + int flags, mode_t mode, int *out_fd, bool expect_no_file); LTTNG_HIDDEN int lttng_trace_chunk_unlink_file(struct lttng_trace_chunk *chunk, -- 2.34.1