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

[Nbd] [PATCH] add support for NBD_CMD_TRIM, update docs (fwd)



I think this was sent to the wrong address

---------- Forwarded Message ----------
Date: 8 September 2011 12:05:46 +0200
From: Paolo Bonzini <pbonzini@...696...>
To: nbd-general@...72...
CC: alex@...872...
Subject: [PATCH] add support for NBD_CMD_TRIM, update docs

Update the protocol documentation, and implement it as a dummy
command in the server.
---
	This part should be quite uncontroversial, and so should
	be patch 2/2 in my kernel series.

	However, patch 1 is basically doing the same as the patch
	from Alex Bligh that introduced NBD_SET_FLAGS, and also
	there are conflicts all over the place.  They're trivial,
	but they are still conflicts.

	Paul, can you please add a pointer somewhere to your kernel
	tree (best would be in MAINTAINERS) so that I can base my patches
	on existing stuff?  Or will you rebase them yourself?  Thanks.

doc/proto.txt |   36 ++++++++++++++++++++++++++++++------
nbd-server.c  |   11 +++++++++++
nbd.h         |    4 +++-
3 files changed, 44 insertions(+), 7 deletions(-)

diff --git a/doc/proto.txt b/doc/proto.txt
index 4869e02..c115157 100644
--- a/doc/proto.txt
+++ b/doc/proto.txt
@@ -26,8 +26,8 @@ server during the handshake.
There are two message types in the data pushing phase: the request, and
the response.

-There are four request types in the data pushing phase: NBD_CMD_READ,
-NBD_CMD_WRITE, NBD_CMD_DISC (disconnect), and NBD_CMD_FLUSH.
+There are five request types in the data pushing phase: NBD_CMD_READ,
+NBD_CMD_WRITE, NBD_CMD_DISC (disconnect), NBD_CMD_FLUSH, NBD_CMD_TRIM.

The request is sent by the client; the response by the server. A request
header consists a 32 bit magic number (magic), a 32 bit field denoting
@@ -38,6 +38,9 @@ data. In the case of NBD_CMD_FLUSH, the offset and length
should  be zero (meaning "flush entire device"); other values are reserved
for future use (e.g. for flushing specific areas without a write).
+Bits 16 and above of the commands are reserved for flags.  Right
+now, the only flag is NBD_CMD_FLAG_FUA (bit 16), "Force unit access".
+
The reply contains three fields: a 32 bit magic number ('magic'), a 32
bit error code ('error'; 0, unless an error occurred in which case it is
the errno of the error on the server side), and the same 64 bit handle
@@ -77,8 +80,7 @@ description of the data that is sent.
S: "NBDMAGIC" (the INIT_PASSWD in the code)
S: 0x00420281861253 (cliserv_magic, a magic number)
S: size of the export in bytes, 64 bit unsigned int
-S: flags, 4 bytes (may have the NBD_FLAG_READ_ONLY flag set if the
-   export is read-only; the rest is reserved)
+S: flags, 4 bytes
S: 124 bytes of zeroes (registered for future use, yes this is
   excessive).

@@ -140,8 +142,7 @@ Once all options are set, the server replies with
information about the  used export:

S: size of the export in bytes, 64 bit unsigned int
-S: flags (16 bits unsigned int, may contain NBD_FLAG_READ_ONLY if the
-   export is read-only)
+S: flags (16 bits unsigned int)
S: 124 bytes of zeroes (forgot to remove that, oops)

The reason that the flags field is 16 bits large and not 32 as in the
@@ -154,3 +155,26 @@ out of flags, the server will set the most significant
flag bit,  signalling that an extra flag field will follow, to which the
client  will have to reply with a flag field of its own before the extra
flags  are sent. This is not yet implemented.
+
+Flag bits
+---------
+
+bit 0 - NBD_FLAG_HAS_FLAGS
+should always be 1
+
+bit 1 - NBD_FLAG_READ_ONLY
+should be set to 1 if the export is read-only
+
+bit 2 - NBD_FLAG_SEND_FLUSH
+should be set to 1 if the server supports NBD_CMD_FLUSH commands
+
+bit 3 - NBD_FLAG_SEND_FUA
+should be set to 1 if the server supports the NBD_CMD_FLAG_FUA flag
+
+bit 4 - NBD_FLAG_ROTATIONAL
+should be set to 1 to let the client schedule I/O accesses as for a
+rotational medium
+
+bit 5 - NBD_FLAG_SEND_TRIM
+should be set to 1 if the server supports NBD_CMD_TRIM commands
+
diff --git a/nbd-server.c b/nbd-server.c
index 81772c2..ef90042 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -163,6 +163,7 @@ int dontfork = 0;
#define F_FUA 256	  /**< Whether server wants FUA to be sent by the client
*/  #define F_ROTATIONAL 512  /**< Whether server wants the client to
implement the elevator algorithm */  #define F_TEMPORARY 1024  /**< Whether
the backing file is temporary and should be created then unlinked */
+#define F_TRIM 2048       /**< Whether server wants TRIM (discard) to be
sent by the client */  GHashTable *children;
char pidfname[256]; /**< name of our PID file */
char pidftemplate[256]; /**< template to be used for the filename of the
PID file */ @@ -830,6 +831,7 @@ GArray* parse_cfile(gchar* f, bool
have_global, GError** e) {  		{ "fua",	FALSE,  PARAM_BOOL,	&(s.flags),
	F_FUA },
		{ "rotational",	FALSE,  PARAM_BOOL,	&(s.flags),		F_ROTATIONAL },
		{ "temporary",	FALSE,  PARAM_BOOL,	&(s.flags),		F_TEMPORARY },
+		{ "trim",	FALSE,  PARAM_BOOL,	&(s.flags),		F_TRIM },
		{ "listenaddr", FALSE,  PARAM_STRING,   &(s.listenaddr),	0 },
		{ "maxconnections", FALSE, PARAM_INT,	&(s.max_connections),	0 },
	};
@@ -1539,6 +1541,8 @@ CLIENT* negotiate(int net, CLIENT *client, GArray*
servers, int phase) {  		flags |= NBD_FLAG_SEND_FUA;
	if (client->server->flags & F_ROTATIONAL)
		flags |= NBD_FLAG_ROTATIONAL;
+	if (client->server->flags & F_TRIM)
+		flags |= NBD_FLAG_SEND_TRIM;
	if (phase & NEG_OLD) {
		/* oldstyle */
		flags = htonl(flags);
@@ -1712,6 +1716,13 @@ int mainloop(CLIENT *client) {
			DEBUG("OK!\n");
			continue;

+		case NBD_CMD_TRIM:
+			/* The kernel module sets discard_zeroes_data == 0,
+			 * so it is okay to do nothing.  */
+			DEBUG ("Ignoring trim request\n");
+			SEND(client->net, reply);
+			continue;
+
		default:
			DEBUG ("Ignoring unknown command\n");
			continue;
diff --git a/nbd.h b/nbd.h
index 875c215..3175773 100644
--- a/nbd.h
+++ b/nbd.h
@@ -33,7 +33,8 @@ enum {
	NBD_CMD_READ = 0,
	NBD_CMD_WRITE = 1,
	NBD_CMD_DISC = 2,
-	NBD_CMD_FLUSH = 3
+	NBD_CMD_FLUSH = 3,
+	NBD_CMD_TRIM = 4
};

#define NBD_CMD_MASK_COMMAND 0x0000ffff
@@ -45,6 +46,7 @@ enum {
#define NBD_FLAG_SEND_FLUSH	(1 << 2)	/* Send FLUSH */
#define NBD_FLAG_SEND_FUA	(1 << 3)	/* Send FUA (Force Unit Access) */
#define NBD_FLAG_ROTATIONAL	(1 << 4)	/* Use elevator algorithm -
rotational media */ +#define NBD_FLAG_SEND_TRIM	(1 << 5)	/* Send TRIM
(discard) */

#define nbd_cmd(req) ((req)->cmd[0])

--
1.7.6



---------- End Forwarded Message ----------



--
Alex Bligh



Reply to: