From 3eb928aad0c1f249830d16d6744a5a23b145a26c Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 14 Nov 2018 16:35:52 -0500 Subject: [PATCH] Fix: consumer: snapshot error return code MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Mathieu Desnoyers Signed-off-by: Jérémie Galarneau --- src/common/kernel-consumer/kernel-consumer.c | 72 +++++++++----------- src/common/sessiond-comm/sessiond-comm.h | 1 + src/common/ust-consumer/ust-consumer.c | 72 ++++++++++---------- 3 files changed, 70 insertions(+), 75 deletions(-) diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c index 68d69bb2a..b890416ca 100644 --- a/src/common/kernel-consumer/kernel-consumer.c +++ b/src/common/kernel-consumer/kernel-consumer.c @@ -124,28 +124,22 @@ int lttng_kconsumer_get_consumed_snapshot(struct lttng_consumer_stream *stream, /* * Take a snapshot of all the stream of a channel + * RCU read-side lock must be held across this function to ensure existence of + * channel. * * Returns 0 on success, < 0 on error */ -int lttng_kconsumer_snapshot_channel(uint64_t key, char *path, - uint64_t relayd_id, uint64_t nb_packets_per_stream, +int lttng_kconsumer_snapshot_channel(struct lttng_consumer_channel *channel, + uint64_t key, char *path, uint64_t relayd_id, uint64_t nb_packets_per_stream, struct lttng_consumer_local_data *ctx) { int ret; - struct lttng_consumer_channel *channel; struct lttng_consumer_stream *stream; DBG("Kernel consumer snapshot channel %" PRIu64, key); rcu_read_lock(); - channel = consumer_find_channel(key); - if (!channel) { - ERR("No channel found for key %" PRIu64, key); - ret = -1; - goto end; - } - /* Splice is not supported yet for channel snapshot. */ if (channel->output != CONSUMER_CHANNEL_MMAP) { ERR("Unsupported output %d", channel->output); @@ -333,15 +327,17 @@ end: /* * Read the whole metadata available for a snapshot. + * RCU read-side lock must be held across this function to ensure existence of + * metadata_channel. * * Returns 0 on success, < 0 on error */ -static int lttng_kconsumer_snapshot_metadata(uint64_t key, char *path, - uint64_t relayd_id, struct lttng_consumer_local_data *ctx) +static int lttng_kconsumer_snapshot_metadata(struct lttng_consumer_channel *metadata_channel, + uint64_t key, char *path, uint64_t relayd_id, + struct lttng_consumer_local_data *ctx) { int ret, use_relayd = 0; ssize_t ret_read; - struct lttng_consumer_channel *metadata_channel; struct lttng_consumer_stream *metadata_stream; assert(ctx); @@ -351,13 +347,6 @@ static int lttng_kconsumer_snapshot_metadata(uint64_t key, char *path, rcu_read_lock(); - metadata_channel = consumer_find_channel(key); - if (!metadata_channel) { - ERR("Kernel snapshot metadata not found for key %" PRIu64, key); - ret = -1; - goto error_no_channel; - } - metadata_stream = metadata_channel->metadata_stream; assert(metadata_stream); pthread_mutex_lock(&metadata_stream->lock); @@ -422,7 +411,6 @@ error_snapshot: cds_list_del(&metadata_stream->send_node); consumer_stream_destroy(metadata_stream, NULL); metadata_channel->metadata_stream = NULL; -error_no_channel: rcu_read_unlock(); return ret; } @@ -900,26 +888,34 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx, } case LTTNG_CONSUMER_SNAPSHOT_CHANNEL: { - if (msg.u.snapshot_channel.metadata == 1) { - ret = lttng_kconsumer_snapshot_metadata(msg.u.snapshot_channel.key, - msg.u.snapshot_channel.pathname, - msg.u.snapshot_channel.relayd_id, ctx); - if (ret < 0) { - ERR("Snapshot metadata failed"); - ret_code = LTTCOMM_CONSUMERD_ERROR_METADATA; - } + struct lttng_consumer_channel *channel; + uint64_t key = msg.u.snapshot_channel.key; + + channel = consumer_find_channel(key); + if (!channel) { + ERR("Channel %" PRIu64 " not found", key); + ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND; } else { - ret = lttng_kconsumer_snapshot_channel(msg.u.snapshot_channel.key, - msg.u.snapshot_channel.pathname, - msg.u.snapshot_channel.relayd_id, - msg.u.snapshot_channel.nb_packets_per_stream, - ctx); - if (ret < 0) { - ERR("Snapshot channel failed"); - ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND; + if (msg.u.snapshot_channel.metadata == 1) { + ret = lttng_kconsumer_snapshot_metadata(channel, key, + msg.u.snapshot_channel.pathname, + msg.u.snapshot_channel.relayd_id, ctx); + if (ret < 0) { + ERR("Snapshot metadata failed"); + ret_code = LTTCOMM_CONSUMERD_SNAPSHOT_FAILED; + } + } else { + ret = lttng_kconsumer_snapshot_channel(channel, key, + msg.u.snapshot_channel.pathname, + msg.u.snapshot_channel.relayd_id, + msg.u.snapshot_channel.nb_packets_per_stream, + ctx); + if (ret < 0) { + ERR("Snapshot channel failed"); + ret_code = LTTCOMM_CONSUMERD_SNAPSHOT_FAILED; + } } } - health_code_update(); ret = consumer_send_status_msg(sock, ret_code); diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h index 5b70a4ac5..f40db1fdd 100644 --- a/src/common/sessiond-comm/sessiond-comm.h +++ b/src/common/sessiond-comm/sessiond-comm.h @@ -169,6 +169,7 @@ enum lttcomm_return_code { LTTCOMM_CONSUMERD_ROTATION_PENDING_LOCAL_FAILED, /* Rotation pending relay failed. */ LTTCOMM_CONSUMERD_ROTATION_PENDING_RELAY_FAILED, /* Rotation pending relay failed. */ LTTCOMM_CONSUMERD_MKDIR_FAILED, /* mkdir has failed. */ + LTTCOMM_CONSUMERD_SNAPSHOT_FAILED, /* snapshot has failed. */ /* MUST be last element */ LTTCOMM_NR, /* Last element */ diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c index 5e34b1a36..73d27b57b 100644 --- a/src/common/ust-consumer/ust-consumer.c +++ b/src/common/ust-consumer/ust-consumer.c @@ -987,15 +987,17 @@ end: /* * Snapshot the whole metadata. + * RCU read-side lock must be held across this function to ensure existence of + * metadata_channel. * * Returns 0 on success, < 0 on error */ -static int snapshot_metadata(uint64_t key, char *path, uint64_t relayd_id, +static int snapshot_metadata(struct lttng_consumer_channel *metadata_channel, + uint64_t key, char *path, uint64_t relayd_id, struct lttng_consumer_local_data *ctx, uint64_t trace_archive_id) { int ret = 0; - struct lttng_consumer_channel *metadata_channel; struct lttng_consumer_stream *metadata_stream; assert(path); @@ -1006,13 +1008,6 @@ static int snapshot_metadata(uint64_t key, char *path, uint64_t relayd_id, rcu_read_lock(); - metadata_channel = consumer_find_channel(key); - if (!metadata_channel) { - ERR("UST snapshot metadata channel not found for key %" PRIu64, - key); - ret = -1; - goto error; - } assert(!metadata_channel->monitor); health_code_update(); @@ -1083,17 +1078,19 @@ error: /* * Take a snapshot of all the stream of a channel. + * RCU read-side lock must be held across this function to ensure existence of + * channel. * * Returns 0 on success, < 0 on error */ -static int snapshot_channel(uint64_t key, char *path, uint64_t relayd_id, +static int snapshot_channel(struct lttng_consumer_channel *channel, + uint64_t key, char *path, uint64_t relayd_id, uint64_t nb_packets_per_stream, struct lttng_consumer_local_data *ctx) { int ret; unsigned use_relayd = 0; unsigned long consumed_pos, produced_pos; - struct lttng_consumer_channel *channel; struct lttng_consumer_stream *stream; assert(path); @@ -1105,12 +1102,6 @@ static int snapshot_channel(uint64_t key, char *path, uint64_t relayd_id, use_relayd = 1; } - channel = consumer_find_channel(key); - if (!channel) { - ERR("UST snapshot channel not found for key %" PRIu64, key); - ret = -1; - goto error; - } assert(!channel->monitor); DBG("UST consumer snapshot channel %" PRIu64, key); @@ -1247,7 +1238,6 @@ error_close_stream: consumer_stream_close(stream); error_unlock: pthread_mutex_unlock(&stream->lock); -error: rcu_read_unlock(); return ret; } @@ -1751,28 +1741,36 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx, } case LTTNG_CONSUMER_SNAPSHOT_CHANNEL: { - if (msg.u.snapshot_channel.metadata) { - ret = snapshot_metadata(msg.u.snapshot_channel.key, - msg.u.snapshot_channel.pathname, - msg.u.snapshot_channel.relayd_id, - ctx, - msg.u.snapshot_channel.trace_archive_id); - if (ret < 0) { - ERR("Snapshot metadata failed"); - ret_code = LTTCOMM_CONSUMERD_ERROR_METADATA; - } + struct lttng_consumer_channel *channel; + uint64_t key = msg.u.snapshot_channel.key; + + channel = consumer_find_channel(key); + if (!channel) { + DBG("UST snapshot channel not found for key %" PRIu64, key); + ret_code = LTTCOMM_CONSUMERD_CHAN_NOT_FOUND; } else { - ret = snapshot_channel(msg.u.snapshot_channel.key, - msg.u.snapshot_channel.pathname, - msg.u.snapshot_channel.relayd_id, - msg.u.snapshot_channel.nb_packets_per_stream, - ctx); - if (ret < 0) { - ERR("Snapshot channel failed"); - ret_code = LTTCOMM_CONSUMERD_CHANNEL_FAIL; + if (msg.u.snapshot_channel.metadata) { + ret = snapshot_metadata(channel, key, + msg.u.snapshot_channel.pathname, + msg.u.snapshot_channel.relayd_id, + ctx, + msg.u.snapshot_channel.trace_archive_id); + if (ret < 0) { + ERR("Snapshot metadata failed"); + ret_code = LTTCOMM_CONSUMERD_SNAPSHOT_FAILED; + } + } else { + ret = snapshot_channel(channel, key, + msg.u.snapshot_channel.pathname, + msg.u.snapshot_channel.relayd_id, + msg.u.snapshot_channel.nb_packets_per_stream, + ctx); + if (ret < 0) { + ERR("Snapshot channel failed"); + ret_code = LTTCOMM_CONSUMERD_SNAPSHOT_FAILED; + } } } - health_code_update(); ret = consumer_send_status_msg(sock, ret_code); if (ret < 0) { -- 2.34.1