On Mon, Mar 25, 2019 at 05:30:32PM +0000, Richard W.M. Jones wrote:
On Sat, Mar 23, 2019 at 12:05:51PM -0500, Eric Blake wrote:
> On 3/20/19 5:11 PM, Richard W.M. Jones wrote:
> > These plugins are both based on the same sparse array structure which
> > supports a simple implementation of extents.
> > ---
>
> > +int
> > +sparse_array_extents (struct sparse_array *sa,
> > + uint32_t count, uint64_t offset,
> > + struct nbdkit_extents *extents)
> > +{
> > + uint32_t n, type;
> > + void *p;
> > +
> > + while (count > 0) {
> > + p = lookup (sa, offset, false, &n, NULL);
> > + if (n > count)
> > + n = count;
>
> Why are we clamping the information we return? I'd move this clamp...
It's a copy and paste from one of the previous sparse_array_*
functions in the same file. I agree that we should return the most
possible information and moving the clamp as you suggest is the way to
do that.
So I tried that, but it introduces a subtle error.
Imagine an allocated RAM disk with a size smaller than the 32K page
size of the sparse array:
nbdkit data data="1" size=512
This will return extents information: [length=32768, type=data].
This doesn't matter for qemu which appears to ignore the extra
information, but it causes a bug if we place the truncate filter on
top:
nbdkit --filter=truncate data data="1" size=512 truncate=64K
This now returns the following extents information which is wrong:
[length=32768, type=data]
[length=32768, type=hole|zero]
If we leave the test where it is then we get the expected extents:
[length=512, type=data]
[length=65536-512, type=hole|zero]
Now it's fair to say this is also caused by the sparse array not
knowing the real size of the plugin. We might pass that information
into the sparse array, but I feel as it's very low cost to query the
sparse array we might as well do the simpler thing.
Oh yes and I found a bug in qemu, will discuss that in another email :-)
Rich.
--
Richard Jones, Virtualization Group, Red Hat
http://people.redhat.com/~rjones
Read my programming and virtualization blog:
http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines. Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v