Re: [Qemu-devel] [RFC PATCH v2 5/6] qcow2: implement bdrv_preallocate

[Fam Zheng]

On 2013年11月27日 18:03, Peter Lieven wrote:
> Am 27.11.2013 07:40, schrieb Fam Zheng:
> > On 2013年11月27日 14:01, Hu Tao wrote:
> > > On Wed, Nov 27, 2013 at 11:01:23AM +0800, Fam Zheng wrote:
> > > > On 2013年11月27日 10:15, Hu Tao wrote:
> > > > > Signed-off-by: Hu Tao <address@hidden>
> > > > > ---
> > > > >    block/qcow2.c | 7 +++++++
> > > > >    1 file changed, 7 insertions(+)
> > > > >
> > > > > diff --git a/block/qcow2.c b/block/qcow2.c
> > > > > index b054a01..a23fade 100644
> > > > > --- a/block/qcow2.c
> > > > > +++ b/block/qcow2.c
> > > > > @@ -2180,6 +2180,12 @@ static int qcow2_amend_options(BlockDriverState *bs,
> > > > >        return 0;
> > > > >    }
> > > > >
> > > > > +static int qcow2_preallocate(BlockDriverState *bs, int64_t offset,
> > > > > +                             int64_t length)
> > > > > +{
> > > > > +    return bdrv_preallocate(bs->file, offset, length);
> > > > > +}
> > > > > +
> > > >
> > > > What's the semantics of .bdrv_preallocate? I think you should map
> > > > [offset, offset + length) to clusters in image file, and then
> > > > forward to bs->file, rather than this direct wrapper.
> > > >
> > > > E.g. bdrv_preallocate(qcow2_bs, 0, cluster_size) should call
> > > > bdrv_preallocate(qcow2_bs->file, offset_off_first_cluster,
> > > > cluster_size).
> > >
> > > You mean data clusters here, right? Is there a single function to get
> > > the offset of the first data cluster?
> >
> > There is a function, qcow2_get_cluster_offset.
> This should return no valid offset as long as the cluster is not allocated.
>
> I think you actually have to "write" all clusters of a qcow2 one by one.
> Eventually this write could be an fallocate call instead of a zero write.

Yes, I was wrong about qcow2_get_cluster_offset. The logic here is more 
like cluster allocation in qcow2_alloc_cluster_offset. Maybe we can 
reuse that.

Fam