Re: [Qemu-devel] [PATCH v12 6/6] qcow2: Add falloc and full preallocation option

[Hu Tao <hutao@cn.fujitsu.com>]

On Fri, Aug 22, 2014 at 01:00:53PM +0200, Kevin Wolf wrote:
> Am 11.07.2014 um 08:10 hat Hu Tao geschrieben:
> > This adds preallocation=falloc and preallocation=full mode to qcow2
> > image creation.
> > 
> > preallocation=full allocates disk space by writing zeros to disk to
> > ensure disk space in any cases.
> > 
> > preallocation=falloc likes preallocation=full, but allocates disk space
> > by posix_fallocate().
> > 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  block/qcow2.c              | 31 ++++++++++++++++++++++++--
> >  tests/qemu-iotests/082.out | 54 +++++++++++++++++++++++-----------------------
> >  2 files changed, 56 insertions(+), 29 deletions(-)
> > 
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index cfba93b..f48e915 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -1593,6 +1593,9 @@ static int preallocate(BlockDriverState *bs)
> >      return 0;
> >  }
> >  
> > +static uint64_t minimal_blob_size(uint64_t ts, int cb, int spcb,
> > +                                  uint64_t overhead);
> > +
> >  static int qcow2_create2(const char *filename, int64_t total_size,
> >                           const char *backing_file, const char *backing_format,
> >                           int flags, size_t cluster_size, PreallocMode prealloc,
> > @@ -1628,6 +1631,29 @@ static int qcow2_create2(const char *filename, int64_t total_size,
> >      Error *local_err = NULL;
> >      int ret;
> >  
> > +    if (prealloc == PREALLOC_MODE_FULL || prealloc == PREALLOC_MODE_FALLOC) {
> > +        int64_t meta_size = 0;
> > +        uint64_t nl2e;
> > +
> > +        total_size = align_offset(total_size, cluster_size);
> 
> I don't think it's a good idea to let the virtual disk size depend on
> whether preallocation is enabled or not. You should always get the same
> rounding (which is rounding up to the next sector boundary).
> 
> Do you need full clusters for your calculations below or what is this
> good for? If so, please use a local variable and leave the value used
> for the bdrv_truncate() call unmodified.

Yes, it is for the calculation. I'll use a local variable for it.

Regards,
Hu