Re: [PATCH 2/3] nbd: Add new qemu:allocation-depth metacontext

2020-09-28 Thread Richard W.M. Jones
On Mon, Sep 28, 2020 at 09:33:22AM -0500, Eric Blake wrote:
> On 9/26/20 2:33 AM, Richard W.M. Jones wrote:
> >On Fri, Sep 25, 2020 at 03:32:48PM -0500, Eric Blake wrote:
> >>+The second is related to exposing the source of various extents within
> >>+the image, with a single context named:
> >>+
> >>+qemu:allocation-depth
> >>+
> >>+In the allocation depth context, bits 0 and 1 form a tri-state value:
> >>+
> >>+bits 0-1 clear: NBD_STATE_DEPTH_UNALLOC, means the extent is 
> >>unallocated
> >>+bit 0 set: NBD_STATE_DEPTH_LOCAL, the extent is allocated in this image
> >>+bit 1 set: NBD_STATE_DEPTH_BACKING, the extent is inherited from a
> >>+   backing layer
> >
> >>From the cover description I imagined it would show the actual depth, ie:
> >
> >  top -> backing -> backing -> backing
> >  depth:   12 3     (0 = unallocated)
> >
> >I wonder if that is possible?  (Perhaps there's something I don't
> >understand here.)
> 
> The real reason I don't want to do a straight depth number is that
> 'qemu-img map' combined with x-dirty-bitmap is still a very
> convenient way to get at bits 0 and 1 (even if it requires
> decoding).  But if we plumb in a way for bdrv_get_status to return
> depth counts (rather than reimplementing the depth count ourselves),
> I would have no problem with returning a struct:
> 
> bits 31-4: the depth of the chain
> bits 3-2: reserved (to make reading hex values easier...)
> bits 1-0: tri-state of unalloc, local, or backing
> 
> where it would look like:
> 
> 0x -> unallocated
> 0x0011 -> depth 1, local
> 0x0022 -> depth 2, from the first backing layer
> 0x0032 -> depth 3, from the second backing layer
> 0x0042 ...

This looks nice too.  However I was only bikeshedding so if any of
this is hard to do then don't worry too much.

Would like to add support for this map to nbdinfo too :-)

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




Re: [PATCH 2/3] nbd: Add new qemu:allocation-depth metacontext

2020-09-28 Thread Eric Blake

On 9/26/20 2:33 AM, Richard W.M. Jones wrote:

On Fri, Sep 25, 2020 at 03:32:48PM -0500, Eric Blake wrote:

+The second is related to exposing the source of various extents within
+the image, with a single context named:
+
+qemu:allocation-depth
+
+In the allocation depth context, bits 0 and 1 form a tri-state value:
+
+bits 0-1 clear: NBD_STATE_DEPTH_UNALLOC, means the extent is unallocated
+bit 0 set: NBD_STATE_DEPTH_LOCAL, the extent is allocated in this image
+bit 1 set: NBD_STATE_DEPTH_BACKING, the extent is inherited from a
+   backing layer



From the cover description I imagined it would show the actual depth, ie:


  top -> backing -> backing -> backing
  depth:   12 3     (0 = unallocated)

I wonder if that is possible?  (Perhaps there's something I don't
understand here.)


The real reason I don't want to do a straight depth number is that 
'qemu-img map' combined with x-dirty-bitmap is still a very convenient 
way to get at bits 0 and 1 (even if it requires decoding).  But if we 
plumb in a way for bdrv_get_status to return depth counts (rather than 
reimplementing the depth count ourselves), I would have no problem with 
returning a struct:


bits 31-4: the depth of the chain
bits 3-2: reserved (to make reading hex values easier...)
bits 1-0: tri-state of unalloc, local, or backing

where it would look like:

0x -> unallocated
0x0011 -> depth 1, local
0x0022 -> depth 2, from the first backing layer
0x0032 -> depth 3, from the second backing layer
0x0042 ...

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org




Re: [PATCH 2/3] nbd: Add new qemu:allocation-depth metacontext

2020-09-26 Thread Vladimir Sementsov-Ogievskiy

26.09.2020 10:33, Richard W.M. Jones wrote:

On Fri, Sep 25, 2020 at 03:32:48PM -0500, Eric Blake wrote:

+The second is related to exposing the source of various extents within
+the image, with a single context named:
+
+qemu:allocation-depth
+
+In the allocation depth context, bits 0 and 1 form a tri-state value:
+
+bits 0-1 clear: NBD_STATE_DEPTH_UNALLOC, means the extent is unallocated
+bit 0 set: NBD_STATE_DEPTH_LOCAL, the extent is allocated in this image
+bit 1 set: NBD_STATE_DEPTH_BACKING, the extent is inherited from a
+   backing layer


 From the cover description I imagined it would show the actual depth, ie:

  top -> backing -> backing -> backing
  depth:   12 3     (0 = unallocated)

I wonder if that is possible?  (Perhaps there's something I don't
understand here.)


That's possible if we want it. Probably the best way is to add *depth parameter to 
bdrv_is_allocated_above (and better on top of my "[PATCH v7 0/5] fix & merge 
block_status_above and is_allocated_above")


--
Best regards,
Vladimir



Re: [PATCH 2/3] nbd: Add new qemu:allocation-depth metacontext

2020-09-26 Thread Vladimir Sementsov-Ogievskiy

25.09.2020 23:32, Eric Blake wrote:

'qemu-img map' provides a way to determine which extents of an image
come from the top layer vs. inherited from a backing chain.  This is
useful information worth exposing over NBD.  There is a proposal to
add a QMP command block-dirty-bitmap-populate which can create a dirty
bitmap that reflects allocation information, at which point
qemu:dirty-bitmap:NAME can expose that information via the creation of
a temporary bitmap, but we can shorten the effort by adding a new
qemu:allocation-depth context that does the same thing without an
intermediate bitmap (this patch does not eliminate the need for that
proposal, as it will have other uses as well).

For this patch, I just encoded a tri-state value (unallocated, from
this layer, from any of the backing layers); we could instead or in
addition report an actual depth count per extent, if that proves more
useful.

Note that this patch does not actually enable any way to request a
server to enable this context; that will come in the next patch.

Signed-off-by: Eric Blake 


Looks good to me overall, need to rebase if patch 01 changed (as I propose or 
in some better way).

--
Best regards,
Vladimir



Re: [PATCH 2/3] nbd: Add new qemu:allocation-depth metacontext

2020-09-26 Thread Richard W.M. Jones
On Fri, Sep 25, 2020 at 03:32:48PM -0500, Eric Blake wrote:
> +The second is related to exposing the source of various extents within
> +the image, with a single context named:
> +
> +qemu:allocation-depth
> +
> +In the allocation depth context, bits 0 and 1 form a tri-state value:
> +
> +bits 0-1 clear: NBD_STATE_DEPTH_UNALLOC, means the extent is unallocated
> +bit 0 set: NBD_STATE_DEPTH_LOCAL, the extent is allocated in this image
> +bit 1 set: NBD_STATE_DEPTH_BACKING, the extent is inherited from a
> +   backing layer

>From the cover description I imagined it would show the actual depth, ie:

 top -> backing -> backing -> backing
 depth:   12 3     (0 = unallocated)

I wonder if that is possible?  (Perhaps there's something I don't
understand here.)

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org