On Thu, 09/15 19:34, Denis V. Lunev wrote:
> They should work very similar, covering same areas if backing store is
> shorter than the image. This change is necessary for the followup patch
> switching to bdrv_get_block_status_above() in mirror to avoid assert
> in check_block.
>
> This change should be made very carefully. Let us assume that we have
> top image and 2 backing stores L0->L1->L2.
Stupid question: which one is top and which are backing?
> L0: --------------
> L1: -------
> L2: -------=======
> The data marked as '=' in L2 should not appear as BDRV_BLOCK_ALLOCATED
> and we should return it as filled in L0 image with properly calculated
> *pnum value.
What '-', '=' and ' ' represent aren't immediately clear to me, could you put a
legend in the message too? Something like:
'-': allocated
'=': unallocated
' ': beyong EOF
>
> Signed-off-by: Denis V. Lunev <d...@openvz.org>
> CC: Stefan Hajnoczi <stefa...@redhat.com>
> CC: Fam Zheng <f...@redhat.com>
> CC: Kevin Wolf <kw...@redhat.com>
> CC: Max Reitz <mre...@redhat.com>
> CC: Jeff Cody <jc...@redhat.com>
> ---
> block/io.c | 25 ++++++++++++++++++++-----
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index 420944d..067d465 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1741,18 +1741,33 @@ static int64_t coroutine_fn
> bdrv_co_get_block_status_above(BlockDriverState *bs,
> BlockDriverState **file)
> {
> BlockDriverState *p;
> - int64_t ret = 0;
> + int64_t ret = 0, res = nb_sectors;
>
> assert(bs != base);
> for (p = bs; p != base; p = backing_bs(p)) {
> - ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum,
> file);
> - if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
> - break;
> + int sc;
> + ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, &sc, file);
> + if (ret < 0) {
> + return ret;
> + } else if (ret & BDRV_BLOCK_ALLOCATED) {
> + *pnum = sc;
> + return ret;
> + }
> +
> + if (res > sc && (p == bs || sector_num + sc < p->total_sectors)) {
> + res = sc;
> }
> +
> /* [sector_num, pnum] unallocated on this layer, which could be only
> * the first part of [sector_num, nb_sectors]. */
> - nb_sectors = MIN(nb_sectors, *pnum);
> + nb_sectors = MIN(nb_sectors, sc);
> +
> + if (nb_sectors == 0) {
> + break;
> + }
> }
> +
> + *pnum = res;
> return ret;
> }
>
> --
> 2.7.4
>
>
