similar to: Supporting sparse disks in nbdkit

On Fri, Mar 08, 2019 at 10:50:35AM -0600, Eric Blake wrote: > > int extents (void *handle, uint32_t count, uint64_t offset, > > uint32_t flags /* always 0? */, > > Unless we support the REQ_ONE flag (where the client specifically wants > only one extent returned). ... > > nbdkit would need to do some massaging on this to get it into the > > right

On Mon, Mar 11, 2019 at 02:53:55PM -0500, Eric Blake wrote: >On 3/11/19 2:44 PM, Richard W.M. Jones wrote: >> On Fri, Mar 08, 2019 at 10:50:35AM -0600, Eric Blake wrote: >>>> int extents (void *handle, uint32_t count, uint64_t offset, >>>> uint32_t flags /* always 0? */, >>> >>> Unless we support the REQ_ONE flag (where the client

Not sure what version we're up to, but this reimplements extents using the new simpler structure described in this thread: https://www.redhat.com/archives/libguestfs/2019-March/msg00077.html I also fixed most of the things that Eric pointed out in the previous review, although I need to go back over his replies and check I've got everything. This needs a bit more testing. However the

I'm not sure exactly which version we're up to, but let's say it's version 4. I'm a lot happier with this version: - all filters have been reviewed and changed where I think that's necessary - can_extents is properly defined and implemented now - NBD protocol is followed - I believe it addresses all previous review points where possible The "only" thing

This has already been pushed upstream. I am simply posting these here so we have a reference in the mailing list in case we find bugs later (as I'm sure we will - it's a complex patch series). Great thanks to Eric Blake for tireless review on this one. It also seems to have identified a few minor bugs in qemu along the way. Rich.

The DF flag is only available to clients that negotiated structured replies, and even then, the spec does not require that it be implemented. However, since our current implementation can't fragment NBD_CMD_READ replies, we trivially implement the flag (by ignoring it); we don't even have to pass it on to the plugins. Enhance some documentation about sparse reads while at it (when we

I want to post this but mainly for discussion and early review. It's not safe for these patches to all go upstream yet (because not all filters have been checked/adjusted), but if any patches were to go upstream then probably 1 & 2 only are safe. File, VDDK, memory and data plugins all work, although I have only done minimal testing on them. The current tests, such as they are, all

On 3/8/19 7:48 AM, Richard W.M. Jones wrote: > I've posted a couple of patches towards the ultimate goal of > implementing NBD_CMD_BLOCK_STATUS / base:allocation in nbdkit. Before > I can do the final patch I think we need to discuss how this would be > exposed to plugins since at the end of the day they need to implement > the feature. Quick thoughts for now; I'll

Second version based on nbdkit_extent* calls, as discussed here: https://www.redhat.com/archives/libguestfs/2019-March/msg00033.html Note in particular there is some subtlety about how filters would work in this implementation. See docs/nbdkit-filter.pod for the details. Rich.

This tentative commit implements extents/can_extents, roughly as discussed in the previous thread here: https://www.redhat.com/archives/libguestfs/2019-March/msg00017.html I can't say that I'm a big fan of having the plugin allocate an extents array. There are no other plugin callbacks currently where we require the plugin to allocate complex data structures (or indeed do any allocation

On 3/20/19 5:11 PM, Richard W.M. Jones wrote: > This pair of calls allows plugins to describe which extents in the > virtual disk are allocated, holes or zeroes. > --- I see you've already made a couple of tweaks to your block-status branch since posting this (skipping 0-legnth add_extent, and coalescing consecutive add_extents with the same type), but here's some more review:

On 3/12/19 12:11 PM, Richard W.M. Jones wrote: > This pair of calls allows plugins to describe which extents in the > virtual disk are allocated, holes or zeroes. > --- > +++ b/docs/nbdkit-filter.pod > +The C<extents_map> parameter passed to this function is empty. True only if any earlier filter also passed in an empty map. Maybe it's worth explicitly mentioning that

On 3/11/19 2:44 PM, Richard W.M. Jones wrote: > On Fri, Mar 08, 2019 at 10:50:35AM -0600, Eric Blake wrote: >>> int extents (void *handle, uint32_t count, uint64_t offset, >>> uint32_t flags /* always 0? */, >> >> Unless we support the REQ_ONE flag (where the client specifically wants >> only one extent returned). > ... >>> nbdkit

I'm not sure which version we're up to now. Anyway I believe this addresses all the points that Eric raised in: https://www.redhat.com/archives/libguestfs/2019-March/msg00038.html https://www.redhat.com/archives/libguestfs/2019-March/msg00040.html In particular: - default state of extents_map is all allocated disk - support hole + non-zero - you can now iterate with bounds -

On Tue, Mar 12, 2019 at 09:04:59AM +0100, Martin Kletzander wrote: > On Mon, Mar 11, 2019 at 02:53:55PM -0500, Eric Blake wrote: > >On 3/11/19 2:44 PM, Richard W.M. Jones wrote: > >>On Fri, Mar 08, 2019 at 10:50:35AM -0600, Eric Blake wrote: > >>>> int extents (void *handle, uint32_t count, uint64_t offset, > >>>> uint32_t flags /*

On 3/19/19 11:34 AM, Richard W.M. Jones wrote: > This pair of calls allows plugins to describe which extents in the > virtual disk are allocated, holes or zeroes. > --- > + void nbdkit_extents_free (struct nbdkit_extents_map *); > + > +Frees an existing extents map. Is this like free() where it is safe to call on NULL? > + > +=head3 Iterating over nbdkit_extents_map

On 11/15/22 01:46, Eric Blake wrote: > The spec was silent on how many extents a server could reply with. > However, both qemu and nbdkit (the two server implementations known to > have implemented the NBD_CMD_BLOCK_STATUS extension) implement a hard > cap, and will truncate the amount of extents in a reply to avoid > sending a client a reply so large that the client would treat it

This filter caches the last result of the extents() call and offers a nice speed-up for clients that only support req_one=1 in combination with plugins like vddk, which has no overhead for returning information for multiple extents in one call, but that call is very time-consuming. Quick test showed that on a fast connection and a sparsely allocated 16G disk with a OS installed `qemu-img map`

This is a cover letter to a series of patches being proposed in tandem to four different projects: - nbd: Document a new NBD_CMD_FLAG_FAST_ZERO command flag - qemu: Implement the flag for both clients and server - libnbd: Implement the flag for clients - nbdkit: Implement the flag for servers, including the nbd passthrough client If you want to test the patches together, I've pushed a

It's easy to use the sh script to demonstrate that nbdkit is inefficiently calling into .get_size, .can_fua, and friends more than necessary. We've also commented on the list in the past that it would be nice to ensure that when filters call into next_ops, they are not violating constraints (as we've have to fix several bugs in the past where we did not have such checking to protect