On 7/8/20 6:28 AM, Richard W.M. Jones wrote:
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;
Found my bug. Unless these are a pointers...
+
+ 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.
I'm fine with an assert.
+ /* 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];
...then this merely copies data,
+ 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);
...and manipulating the copy doesn't affect what gets returned. With
that fixed, my tests are now working as desired.
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.
I'll add a comment somewhere why intersection is correct.
All looks good to me, so ACK.
Rich.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs