[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

[PATCH 2/7] nbd-server, nbd-trdump: Add support for logging actual data



The datalog generated by nbd-server contains only the requests
received by the server, not the actual data to be written.

This patch adds support to write the actual data.

As details:
- It is configurable, the default behavior is not changed.
- It adds a NBD_CMD_FLAG_DATALOG flag: I think this is the simplest
  way to mark that actual data wil follow in the log.
  As clients must not use a feature that is not exposed by the server,
  and as the server fails commands with unexpected flags, the approach
  should be safe. Obviously, if the protocol is extended so that 14
  flags are needed, then it will bite us.
- It is an incompatible change: Current nbd-trdump utilities
  will just fail/produce bad output when called with a new
  log file, without a proper error message.
- The change does not add a header to the datalog. I.e.:
  If we sometime in the future extend the log again, it will
  be again an incompatible change without a proper error
  message
- nbd-trdump supports to dump also the messages sent by the
  server. Unfortunately, the current server does not log
  the sent messages. This change does not fix this.

Known bugs:
Locking is missing. If multiple clients connect, then the data log
will be unusable.

Plan: Use a named posix semaphore (sem_open()).
Given the multi-process, multi-thread model, with a single fd shared
by everyone, this is probably simpler than trying to find a reliable
flock()/fcntl()/pthread_mutex_lock() combination.
Alternative: shm_open()+a shared pthread_mutex.

Signed-off-by: Manfred Spraul <manfred.spraul@de.bosch.com>
---
 nbd-helper.h |  4 ++++
 nbd-server.c | 20 +++++++++++++++++++-
 nbd-trdump.c | 13 +++++++++++++
 nbdsrv.h     |  1 +
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/nbd-helper.h b/nbd-helper.h
index 7b7b75d..0d0d1b9 100644
--- a/nbd-helper.h
+++ b/nbd-helper.h
@@ -3,6 +3,10 @@
 
 #include "nbd.h"
 
+/* Constants and macros */
+
+#define NBD_CMD_FLAG_DATALOG ((1 << 14) << NBD_CMD_SHIFT)
+
 /* Functions */
 
 /**
diff --git a/nbd-server.c b/nbd-server.c
index 3c3589e..8c1fba0 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -786,6 +786,7 @@ GArray* parse_cfile(gchar* f, struct generic_conf *const genconf, bool expect_ge
 		{ "rotational",	FALSE,  PARAM_BOOL,	&(s.flags),		F_ROTATIONAL },
 		{ "temporary",	FALSE,  PARAM_BOOL,	&(s.flags),		F_TEMPORARY },
 		{ "trim",	FALSE,  PARAM_BOOL,	&(s.flags),		F_TRIM },
+		{ "datalog",	FALSE,  PARAM_BOOL,	&(s.flags),		F_DATALOG },
 		{ "listenaddr", FALSE,  PARAM_STRING,   &(s.listenaddr),	0 },
 		{ "maxconnections", FALSE, PARAM_INT,	&(s.max_connections),	0 },
 		{ "force_tls",	FALSE,	PARAM_BOOL,	&(s.flags),		F_FORCEDTLS },
@@ -982,6 +983,15 @@ GArray* parse_cfile(gchar* f, struct generic_conf *const genconf, bool expect_ge
 			g_key_file_free(cfile);
 			return NULL;
 		}
+		/* We can't mix datalog and splice. */
+		if ((s.flags & F_DATALOG) && (s.flags & F_SPLICE)) {
+			g_set_error(e, NBDS_ERR, NBDS_ERR_CFILE_INVALID_SPLICE,
+				    "Cannot mix datalog with splice for an export in group %s",
+				    groups[i]);
+			g_array_free(retval, TRUE);
+			g_key_file_free(cfile);
+			return NULL;
+		}
 		/* Don't need to free this, it's not our string */
 		virtstyle=NULL;
 		/* Don't append values for the [generic] group */
@@ -2692,7 +2702,7 @@ static void handle_request(gpointer data, gpointer user_data) {
 	struct nbd_reply rep;
 	int err = EINVAL;
 
-	if(flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE)) {
+	if(flags & ~(NBD_CMD_FLAG_FUA | NBD_CMD_FLAG_NO_HOLE | NBD_CMD_FLAG_DATALOG)) {
 		msg(LOG_ERR, "E: received invalid flag %d on command %d, ignoring", flags, type);
 		goto error;
 	}
@@ -2764,6 +2774,11 @@ static int mainloop_threaded(CLIENT* client) {
 
 		socket_read(client, req, sizeof(struct nbd_request));
 		if(client->transactionlogfd != -1) {
+			if(((ntohl(req->type) & NBD_CMD_MASK_COMMAND) == NBD_CMD_WRITE) &&
+					(client->server->flags & F_DATALOG) &&
+					!(client->server->flags & F_SPLICE)) {
+				req->type = htonl(ntohl(req->type)|NBD_CMD_FLAG_DATALOG);
+			}
 			writeit(client->transactionlogfd, req, sizeof(struct nbd_request));
 		}
 
@@ -2786,6 +2801,9 @@ static int mainloop_threaded(CLIENT* client) {
 			else
 #endif
 				socket_read(client, pkg->data, req->len);
+
+			if (req->type & NBD_CMD_FLAG_DATALOG)
+				writeit(client->transactionlogfd, pkg->data, req->len);
 		}
 		if(req->type == NBD_CMD_DISC) {
 			finalize_client(client);
diff --git a/nbd-trdump.c b/nbd-trdump.c
index 8131698..ae32adc 100644
--- a/nbd-trdump.c
+++ b/nbd-trdump.c
@@ -19,6 +19,9 @@
 #include "nbd.h"
 #include "nbd-helper.h"
 
+#define BUFSIZE	131072
+static char tmpbuf[BUFSIZE];
+
 static inline void doread(int f, void *buf, size_t len) {
         ssize_t res;
 
@@ -78,6 +81,16 @@ int main(int argc, char**argv) {
 			       (command & NBD_CMD_FLAG_FUA)?"FUA":"NONE",
 			       (long long unsigned int) offset,
 			       len);
+			if (command & NBD_CMD_FLAG_DATALOG) {
+				while (len > 0) {
+					uint32_t tmplen = len;
+
+					if (tmplen > BUFSIZE)
+						tmplen = BUFSIZE;
+					doread(readfd, tmpbuf, tmplen);
+					len -= tmplen;
+				}
+			}
 			
 			break;
 		case NBD_REPLY_MAGIC:
diff --git a/nbdsrv.h b/nbdsrv.h
index 6a20c9b..b84e26a 100644
--- a/nbdsrv.h
+++ b/nbdsrv.h
@@ -155,6 +155,7 @@ typedef enum {
 #define F_FORCEDTLS 16384 /**< TLS is required, either for the server as a whole or for a given export */
 #define F_SPLICE 32768	  /**< flag to tell us to use splice for read/write operations */
 #define F_WAIT 65536      /**< flag to tell us to wait for file creation */
+#define F_DATALOG 131072  /**< flag to tell us that the transaction log shall contain the written data */
 
 /* Functions */
 
-- 
2.33.1


Reply to: