oll3 / nbd-async Goto Github PK

#4's first commit removed the Send requirement on BlockDevice. This makes sense, because there's no real reason it needs to be send...but it also has a very unfortunate consequence: BlockDevice can never be Send.

As a result, serve_nbd's returned Future isn't Send, which heavily limits where it can be used. In particular, I was trying to do this:

tokio::spawn(start_nbd(...));

But, because start_nbd's result isn't Send, it's not possible without resorting to using a LocalSet, which is...well, pretty ugly.

Unfortunately, I'm not sure if there's a particularly "clean" way to solve this. I think if send is a crate-level feature, then you could do:

#[cfg_attr(feature = "send", async_trait)]
#[cfg_attr(not(feature = "send")), async_trait(?Send))]
trait BlockDevice { ... }

i.e. BlockDevice is only Send if the corresponding feature is active. This does make it a workspace-wide control, but it's the only non-insane option I can think of. EDIT: well turns out you need to copy the entire BlockDevice definition, with one behind cfg(feature = "send") that's trait BlockDevice: Send, didn't notice before because rust-analyzer wasn't communicating the errors to me 😅

From the documentation for NBD_CMD_READ:

If an error occurs, the server SHOULD set the appropriate error code
in the error field. The server MAY then initiate a hard disconnect.
If it chooses not to, it MUST NOT send any payload for this request.

So it seems to me that after setting reply.error, the reply_buf should be resized back to nbd::SIZE_OF_REPLY before sending the reply.

Thank you for building this, it looks great!

One request: would you consider separating the interface for responding to NBD requests given a BlockDevice implementation from the interface for attaching an NBD socket to the local system? That would make it easy to hook the former up to a server socket, and to use the latter to attach a remote socket to a local NBD device.