From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56555) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ywtbm-00033q-V7 for qemu-devel@nongnu.org; Mon, 25 May 2015 10:44:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ywtbh-00046M-HN for qemu-devel@nongnu.org; Mon, 25 May 2015 10:44:02 -0400 Message-ID: <55633519.9030407@redhat.com> Date: Mon, 25 May 2015 16:43:37 +0200 From: Paolo Bonzini MIME-Version: 1.0 References: <1432266060-22104-1-git-send-email-famz@redhat.com> <1432266060-22104-2-git-send-email-famz@redhat.com> In-Reply-To: <1432266060-22104-2-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 1/8] block: Add "base" option to bdrv_get_block_status List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng , qemu-devel@nongnu.org Cc: Kevin Wolf , qemu-block@nongnu.org, Jeff Cody , qemu-stable@nongnu.org, Stefan Hajnoczi , jsnow@redhat.com, wangxiaolong@ucloud.cn On 22/05/2015 05:40, Fam Zheng wrote: > +{ > + BlockDriverState *p; > + int64_t ret; > + > + assert(bs != base); > + for (p = bs; p != base; p = p->backing_hd) { > + ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum); It's a bit ugly to have different parameters for bdrv_get_block_status and bdrv_co_get_block_status. > + if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) { > + break; > + } > + } > + return ret; > +} > + I think BDRV_BLOCK_ALLOCATED will always be true if passing NULL as the base, so patch 3 still needs a tweak. But, more important, you need extra logic to correct the output value of pnum, as in bdrv_is_allocated_above. If you want, you can also reimplemented bdrv_is_allocated_above on top of bdrv_get_block_status. Paolo