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

[Nbd] Cleaning up mainloop in nbd-server



mainloop in nbd-server currently does this:

               if (command==NBD_CMD_DISC) {
                       msg2(LOG_INFO, "Disconnect request received.");
			...
               }

               len = ntohl(request.len);

               if (request.magic != htonl(NBD_REQUEST_MAGIC))
                       err("Not enough magic.");

               if (len > BUFSIZE - sizeof(struct nbd_reply)) {
                       currlen = BUFSIZE - sizeof(struct nbd_reply);
msg2(LOG_INFO, "oversized request (this is not a problem)");
               } else {
                       currlen = len;
               }

		[memcpy handle]

		[check offset if appropriate]

		[handle command with lots of "if"]

I tried to clear this up somewhat half-heartedly then reverted, but I may
have another go.

However, does the protocol *really* require that we check for
NBD_CMD_DISC prior to checking for a correct magic number? If not,
I think a more logical sequence would be:


               if (request.magic != htonl(NBD_REQUEST_MAGIC))
                       err("Not enough magic.");

		[memcpy handle]

               len = ntohl(request.len);

		[check offset if appropriate]

		[handle command with switch, including disconnect]

		[do oversize check in NBD_CMD_READ case: block]

This does mean we do a little work preparing a reply for NBD_DISCONNECT
which never gets sent (an 8 byte memcpy) but I think that's a small
price to pay for a more logical code flow.

--
Alex Bligh



Reply to: