Re: [Nbd] Cleaning up mainloop in nbd-server

[Wouter Verhelst <w@...112...>]

On Fri, May 27, 2011 at 06:47:49PM +0100, Alex Bligh wrote:
> 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?

It most certainly does not, and indeed it is much more logical (the
"magic" is supposed to guard against desyncing of the protocol; now if
we don't check for that, then what is it for really?)

Will have an in-depth review of your patch tomorrow (it's too late (or
should I say, early?) right now to do that properly), and if it looks
fine, will merge.

Thanks,

-- 
The volume of a pizza of thickness a and radius z can be described by
the following formula:

pi zz a