Re: [Libguestfs] [nbdkit PATCH 2/3] extents: Add nbdkit_extents_aligned()

On Tue, Jul 07, 2020 at 05:22:46PM -0500, Eric Blake wrote: [...] > +/* Compute aligned extents on behalf of a filter. */ > +int > +nbdkit_extents_aligned (struct nbdkit_next_ops *next_ops, > + nbdkit_backend *nxdata, > + uint32_t count, uint64_t offset, > + uint32_t flags, uint32_t align, > + struct nbdkit_extents *exts, int *err) > +{ > + size_t i; > + struct nbdkit_extent e, e2;

> + > + if (!IS_ALIGNED(count | offset, align)) { > + nbdkit_error ("nbdkit_extents_aligned: unaligned request"); > + *err = EINVAL; > + return -1; > + } I wonder if this also should be an assert? This is less clear to me than the vector case however.

> + /* Perform an initial query, then scan for the first unaligned extent. */ > + if (next_ops->extents (nxdata, count, offset, flags, exts, err) == -1) > + return -1; > + for (i = 0; i < exts->extents.size; ++i) { > + e = exts->extents.ptr[i];

> + if (!IS_ALIGNED(e.length, align)) { > + /* If the unalignment is past align, just truncate and return early */ > + if (e.offset + e.length > offset + align) { > + e.length = ROUND_DOWN (e.length, align);

So we're intersecting (&) the types defined as: #define NBDKIT_EXTENT_HOLE (1<<0) /* Same as NBD_STATE_HOLE */ #define NBDKIT_EXTENT_ZERO (1<<1) /* Same as NBD_STATE_ZERO */ If all extents are holes, then it's a hole. If all extents are zero, then it's a zero. Otherwise it's non-zero data. This seems correct.

All looks good to me, so ACK. Rich.