[Libguestfs] [PATCH nbdkit 7/8] vddk: Implement extents.

[Richard W.M. Jones]

On Sat, Mar 23, 2019 at 02:29:54PM -0500, Eric Blake wrote:
> On 3/20/19 5:11 PM, Richard W.M. Jones wrote:
...
> > +    for (i = 0; i < block_list->numBlocks; ++i) {
> > +      uint64_t offset, length;
> > +
> > +      offset = block_list->blocks[i].offset * VIXDISKLIB_SECTOR_SIZE;
> > +      length = block_list->blocks[i].length * VIXDISKLIB_SECTOR_SIZE;
> > +
> > +      /* The query returns blocks.  We must insert holes between the
> > +       * blocks as necessary.
> > +       */
> > +      if (position < offset) {
> > +        if (nbdkit_add_extent (extents,
> > +                               offset, length,
> > +                               NBDKIT_EXTENT_HOLE | NBDKIT_EXTENT_ZERO) == -1)
> > +          goto error_in_add;
> > +      }
> > +
> > +      if (nbdkit_add_extent (extents,
> > +                             offset, length, 0 /* allocated data */) == -1) {
> > +      error_in_add:
> > +        DEBUG_CALL ("VixDiskLib_FreeBlockList", "block_list");
> > +        VixDiskLib_FreeBlockList (block_list);
> > +        return -1;
> > +      }
> > +
> > +      position = offset + length;
> 
> This inner loop might be long; you could add this:
> 
> if (req_one)
>   break;

The issue in practice is that VixDiskLib_QueryAllocatedBlocks is very
expensive (because it's a remote procedure call), but we expect
calling nbdkit_add_extent should be very cheap.

However I'll experiment with breaking earlier from the inner loop.  I
need to do a lot more testing on this patch anyway.

Rich.

> > +    }
> > +    DEBUG_CALL ("VixDiskLib_FreeBlockList", "block_list");
> > +    VixDiskLib_FreeBlockList (block_list);
> > +
> > +    start_sector += nr_sectors;
> > +
> > +    if (req_one)
> > +      break;
> 
> just as you break the outer loop here, to make REQ_ONE slightly faster.
> 
> Otherwise looks sane. m not in a position to test it, but if 'qemu-img
> map --output=json nbd://...' pointing to your server matches what you
> expect for vddk, that's a good sign.
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.           +1-919-301-3226
> Virtualization:  qemu.org | libvirt.org
> 




-- 
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