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

[Richard W.M. Jones]

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);
> +        exts->extents.size = i + !!e.length;
> +        exts->next = e.offset + e.length;
> +        break;
> +      }
> +
> +      /* Otherwise, coalesce until we have at least align bytes, which
> +       * may require further queries.
> +       */
> +      assert (i == 0);
> +      while (e.length < align) {
> +        if (exts->extents.size > 1) {
> +          e.length += exts->extents.ptr[1].length;
> +          e.type &= exts->extents.ptr[1].type;
> +          extents_remove (&exts->extents, 1);
> +        }
> +        else {
> +          /* The plugin needs a fresh extents object each time, but
> +           * with care, we can merge it into the callers' extents.
> +           */
> +          extents tmp;
> +          CLEANUP_EXTENTS_FREE struct nbdkit_extents *extents2 = NULL;
> +
> +          extents2 = nbdkit_extents_new (e.offset + e.length, offset + align);
> +          if (next_ops->extents (nxdata, offset + align - e.length,
> +                                 e.offset + e.length,
> +                                 flags & ~NBDKIT_FLAG_REQ_ONE,
> +                                 extents2, err) == -1)
> +            return -1;
> +          e2 = extents2->extents.ptr[0];
> +          assert (e2.offset == e.offset + e.length);
> +          e2.offset = e.offset;
> +          e2.length += e.length;
> +          e2.type &= e.type;

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.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html