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

Re: [Nbd] [PATCH] Don't error on NBD_FLAG_CMD_FUA on non-writes



On 6 Apr 2016, at 19:39, Wouter Verhelst <w@...112...> wrote:

> On Wed, Apr 06, 2016 at 04:37:23PM +0100, Alex Bligh wrote:
>> Note Eric's also fixed another bug. The line should
>> previously have only tested the FUA bit, rather than do
>> a word comparison.
> 
> How so?

The old code had:

  if(flags != 0 && (type != NBD_CMD_WRITE || flags != NBD_CMD_FLAG_FUA)) {
      ... cause an error ...
  }

I was thinking the 'flags != NBD_CMD_FLAG_FUA' should have been
'!(flags & NBD_CMD_FLAG_FUA)' and 'flags != 0' should have been
'(flags & ~NBD_CMD_FLAG_FUA)' etc. but it probably makes no difference
as the reference implementation currently only supports one command
flag I think.

Eric is however future-proofing it.

(Eric's patch below for ease of reference)

-- 
Alex Bligh


---
nbd-server.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nbd-server.c b/nbd-server.c
index b222a11..4edb883 100644
--- a/nbd-server.c
+++ b/nbd-server.c
@@ -1523,7 +1523,7 @@ static void handle_request(gpointer data, gpointer user_data) {
	uint32_t flags = package->req->type & ~NBD_CMD_MASK_COMMAND;
	struct nbd_reply rep;

-	if(flags != 0 && (type != NBD_CMD_WRITE || flags != NBD_CMD_FLAG_FUA)) {
+	if(flags & ~NBD_CMD_FLAG_FUA) {
		msg(LOG_ERR, "E: received invalid flag %d on command %d, ignoring", flags, type);
		goto error;
	}
-- 
2.5.5


Reply to: