From: David Goulet <dgoulet@efficios.com>
Date: Mon, 17 Sep 2012 15:57:07 +0000 (-0400)
Subject: Fix: Consumer return value check after mmap/splice
X-Git-Tag: v2.1.0-rc4~27
X-Git-Url: https://git.lttng.org/?p=lttng-tools.git;a=commitdiff_plain;h=91dfef6e2f50460b9597fc8a8186949c48557c14

Fix: Consumer return value check after mmap/splice

With the feature of not sending the trace file padding over the network
for streaming, the return value of mmap or splice failed to match the
passed value when the trace is written on disk since we ignore the
padding for that case.

However, this was simply to print an error message so no behavior is
changed with this patch.

Detail comments were added to explain this "complex" return value check.

Signed-off-by: David Goulet <dgoulet@efficios.com>
---

diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c
index cd814b8a7..f910f033d 100644
--- a/src/common/kernel-consumer/kernel-consumer.c
+++ b/src/common/kernel-consumer/kernel-consumer.c
@@ -337,8 +337,12 @@ ssize_t lttng_kconsumer_read_subbuffer(struct lttng_consumer_stream *stream,
 		padding = 0;
 
 		/* splice the subbuffer to the tracefile */
-		ret = lttng_consumer_on_read_subbuffer_splice(ctx, stream,
-				subbuf_size, padding);
+		ret = lttng_consumer_on_read_subbuffer_splice(ctx, stream, subbuf_size,
+				padding);
+		/*
+		 * XXX: Splice does not support network streaming so the return value
+		 * is simply checked against subbuf_size and not like the mmap() op.
+		 */
 		if (ret != subbuf_size) {
 			/*
 			 * display the error but continue processing to try
@@ -364,15 +368,22 @@ ssize_t lttng_kconsumer_read_subbuffer(struct lttng_consumer_stream *stream,
 		padding = len - subbuf_size;
 
 		/* write the subbuffer to the tracefile */
-		ret = lttng_consumer_on_read_subbuffer_mmap(ctx, stream,
-				subbuf_size, padding);
-		if (ret != subbuf_size) {
+		ret = lttng_consumer_on_read_subbuffer_mmap(ctx, stream, subbuf_size,
+				padding);
+		/*
+		 * The mmap operation should write subbuf_size amount of data when
+		 * network streaming or the full padding (len) size when we are _not_
+		 * streaming.
+		 */
+		if ((ret != subbuf_size && stream->net_seq_idx != -1) ||
+				(ret != len && stream->net_seq_idx == -1)) {
 			/*
-			 * display the error but continue processing to try
-			 * to release the subbuffer
+			 * Display the error but continue processing to try to release the
+			 * subbuffer
 			 */
-			ERR("Error writing to tracefile (ret: %zd != len: %lu",
-					ret, subbuf_size);
+			ERR("Error writing to tracefile "
+					"(ret: %zd != len: %lu != subbuf_size: %lu)",
+					ret, len, subbuf_size);
 		}
 		break;
 	default:
diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index e7d6dd4e7..8ab2b819d 100644
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -452,12 +452,20 @@ int lttng_ustconsumer_read_subbuffer(struct lttng_consumer_stream *stream,
 	padding = len - subbuf_size;
 	/* write the subbuffer to the tracefile */
 	ret = lttng_consumer_on_read_subbuffer_mmap(ctx, stream, subbuf_size, padding);
-	if (ret != subbuf_size) {
+	/*
+	 * The mmap operation should write subbuf_size amount of data when network
+	 * streaming or the full padding (len) size when we are _not_ streaming.
+	 */
+	if ((ret != subbuf_size && stream->net_seq_idx != -1) ||
+			(ret != len && stream->net_seq_idx == -1)) {
 		/*
-		 * display the error but continue processing to try
-		 * to release the subbuffer
+		 * Display the error but continue processing to try to release the
+		 * subbuffer
 		 */
-		ERR("Error writing to tracefile (expected: %ld, got: %ld)", ret, len);
+		ERR("Error writing to tracefile "
+				"(ret: %zd != len: %lu != subbuf_size: %lu)",
+				ret, len, subbuf_size);
+
 	}
 	err = ustctl_put_next_subbuf(handle, buf);
 	assert(err == 0);