Re: [Nbd] libnbd

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

On 29-04-13 11:45, Goswin von Brederlow wrote:
> On Thu, Apr 25, 2013 at 06:39:21PM +0200, Wouter Verhelst wrote:
>> No. nbd-client is, and will always be, a very small footprint program,
>> since it needs to be able to run from initrd and similar things. I
>> intend to use libglib's helper functions extensively inside libnbd; that
>> would incur a massive footprint increase. That simply is Not An Option(TM).
>>
>> (I might add much of what nbd-client does as separate calls in libnbd
>> for the benefit of others, but again, that won't be where my initial
>> focus would lie)
> 
> There could be a small footprint libnbd-client.so.

Yes. Eventually. Again, that won't be my initial focus. Note the use of
"initial" in there. Also, if you want to, I won't refuse patches.

> Personally i don't really care about the size of the executable +
> libraries. That is just mmaped. Parts that are not used don't take up
> memory. Any increase in memory will be temporary only till it starts
> the kernel part too.

initrd bloat is a real problem. If there's too much in there, it won't
boot anymore on systems with low memory.

Since nbd is fairly popular in LTSP settings, which is often used to
recycle older systems, this is a real concern.

>>> If the server uses a state machine then the alloc_request() callback
>>> would create a new one. It could be declared as
>>>
>>> struct my_state_machine {
>>>   struct nbd_request; // must be first
>>>   int my_state;
>>>   void * foo;
>>> };
>>>
>>> The libnbd would then will in the struct nbd_request and eventually
>>> call handle_request(). That then starts the state machine.
>>
>> I'm not sure I understand what you're on about here. Can you elaborate?
> 
> The interface for the main loop would require you to pass something to
> handle requests. You would have for example these 3 callbacks:
> 
> struct request_ops {
>   struct nbd_request * (*alloc_request)();
>   void (*handle_request)(struct nbd_request *);
>   void (*free_request)(struct nbd_request *);
> };
>
> The struct request_ops would be the visible interface between the main
> loop and the request handling. People can implement their own or use
> the one from libnbd. 

Ah, I see.

It's totally different from what I had in mind, but I suppose that could
work.

[...]
>>> I just wouldn't require that requests are handled as state machine.
>>> The interface between receiving and sending requests and handling them
>>> don't interact much. It's just create, handle, reply, free. 4 little
>>> functions that will be called in that order.
>>
>> Well, here's the thing: "handle" in current nbd-server consists of:
>> - Check the type of the request (read/write/trim/flush/disconnect)
>> - read data from the socket, if this is a write request
> 
> That is part of parsing the protocol and as such part of the select
> loop.

Yes, perhaps.

>> - figure out if we're using copy-on-write; if so, redirect the relevant
>> parts of the request to the diff file
>> - figure out if we're using the multiple file option; if so, distribute
>> the request to the relevant files from our "multiple" export.
>> - Read data from the correct file into a buffer
> 
> That is part of the request handling.

Which is a lot to do in "one little function". Too much if we want to
allow people to easily replace part of that.

[...]
>> That's a lot of stuff to do for a "little" function. The current code
>> just has the function which implements the first bullet point in the
>> above list call the next function. That works, I suppose, but it's
>> extremely non-flexible.
>>
>> If we're going to allow people to replace (or disable?) some of that
>> functionality, they can't be directly calling eachother anymore. I think
>> a state machine there is the most flexible way to make that possible
>> (although I could certainly be wrong about that).
> 
> The "little" function handle_request() would be, in your case, start a
> complex state machine. The implementation of the state machine can
> have before/after hooks and replacable states and such. That is all fine.
> 
> But I would like to see a clear seperation of the select loop and
> protocol parsing and handling requests.

Of course, that's the whole plan.

In my view, the select loop would:
- Read a request header into a struct nbd_request, and parse it (i.e.,
call ntoh* on the right variables of the struct that we read the header
into)
- Figure out if the request is followed by data; if so, read that data
from the socket
- Inform the correct state machine that some data is available, and move
on to the next request

Everything else would be done by functions called by the state machine.

[...]

-- 
Copyshops should do vouchers. So that next time some bureaucracy
requires you to mail a form in triplicate, you can mail it just once,
add a voucher, and save on postage.