Re: [Libguestfs] [libnbd PATCH 4/4] api: Add DF flag support for pread

On 5/28/19 4:26 AM, Richard W.M. Jones wrote: Summarizing an IRC conversation, I doubt we will ever see any serious users request the DF flag for nbd_[aio_]pread. The only thing the flag would ever do for such readers is turn what would otherwise be a successful read into an EOVERFLOW failure, or to possibly make the connection slower (because getting a single-buffer response required more network traffic than what would otherwise be possible by sending holes). So the consensus would be that while we might add support for passing the flag, we don't have to go out of our way to document it for nbd_pread (or maybe even document that it is pointless), and instead, focus on: ...what exactly we want to support here. My observation is that right now, libnbd does NOT validate server compliance on structured read (we don't know if a server populated the entire buffer, or whether it sent overlapping chunks). But having some sort of callback that gets invoked on each chunk would allow the client to add in such checking; we could even provide a utility function in the library to serve as such a callback if our checking is sufficient for the client's purposes. Something like: nbd_pread_callback(nbd, buf, count, offset, opaque, callback, flags) where we still read off the wire into buf (to avoid ping-ponging a read into a temporary buf that the callback then has to memcpy() into the real buf), but call the callback as soon as each chunk is received: callback(opaque, buf, count, offset, status) telling the user which subset of buf was just populated, and where status is data, hole, or error. The signature may still need tweaking. Or we may even want to let the user register a set of callbacks, where a different callback is invoked for different chunk types: set = nbd_callback_set_create(nbd, NBD_CMD_READ, opaque, default_cb, default_error_cb); nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_DATA, opaque, cb); nbd_callback_set_add(nbd, set, NBD_REPLY_TYPE_OFFSET_HOLE, opaque, cb); nbd_pread_callback(nbd, buf, count, offset, set, flags); The idea of registering a set of callbacks to handle a particular integer command id may work well for other extensions as well; particularly if we encounter a case where an app wants to use libnbd for the groundwork (setting up a tls connection handshake) but still implement their own non-standard NBD extensions that the server will understand (where the libnbd state machine parses the chunks off the wire, but the client then interprets those chunks). Which means we may also need hooks for a client to inject other NBD_OPT sequences into the state machine beyond what we know natively. Various ideas still floating around, it may be a while before we actually have code to match those ideas in order to pick what works best. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org