Le Friday 24 Jan 2014 à 17:32:40 (+0800), Hu Tao a écrit :
> On Thu, Jan 23, 2014 at 06:02:08PM +0100, Benoît Canet wrote:
> > Le Thursday 23 Jan 2014 à 11:04:05 (+0800), Hu Tao a écrit :
> > > n_start can be actually calculated from offset. The number of
> > > sectors to be allocated(n_end - n_start) can be passed in in
> > > num. By removing n_start and n_end, we can save two parameters.
> > >
> > > The side effect is there is a bug in qcow2.c:preallocate() that
> > > passes incorrect n_start to qcow2_alloc_cluster_offset() is
> > > fixed. The bug can be triggerred by a larger cluster size than
> > > the default value(65536), for example:
> > >
> > > ./qemu-img create -f qcow2 \
> > > -o 'cluster_size=131072,preallocation=metadata' file.img 4G
> > >
> > > Reviewed-by: Max Reitz <mre...@redhat.com>
> > > Signed-off-by: Hu Tao <hu...@cn.fujitsu.com>
> > > ---
> > > block/qcow2-cluster.c | 14 ++++++--------
> > > block/qcow2.c | 11 +++--------
> > > block/qcow2.h | 2 +-
> > > trace-events | 2 +-
> > > 4 files changed, 11 insertions(+), 18 deletions(-)
> > >
> > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> > > index 8534084..c57f39d 100644
> > > --- a/block/qcow2-cluster.c
> > > +++ b/block/qcow2-cluster.c
> > > @@ -1182,7 +1182,7 @@ fail:
> > > * Return 0 on success and -errno in error cases
> > > */
> > > int qcow2_alloc_cluster_offset(BlockDriverState *bs, uint64_t offset,
> > > - int n_start, int n_end, int *num, uint64_t *host_offset, QCowL2Meta
> > > **m)
> > > + int *num, uint64_t *host_offset, QCowL2Meta **m)
> > > {
> > > BDRVQcowState *s = bs->opaque;
> > > uint64_t start, remaining;
> > > @@ -1190,15 +1190,13 @@ int qcow2_alloc_cluster_offset(BlockDriverState
> > > *bs, uint64_t offset,
> > > uint64_t cur_bytes;
> > > int ret;
> > >
> > > - trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset,
> > > - n_start, n_end);
> > > + trace_qcow2_alloc_clusters_offset(qemu_coroutine_self(), offset,
> > > *num);
> > >
> > > - assert(n_start * BDRV_SECTOR_SIZE == offset_into_cluster(s, offset));
> > > - offset = start_of_cluster(s, offset);
> > > + assert((offset & ~BDRV_SECTOR_MASK) == 0);
> >
> > Why replace something that would round gently an unaligned offset
> > (start_of_cluster) by an assert that would make QEMU exit ?
>
> It is equivalent to the removed assert().
Oh sorry I didn't see the removed assert() when reviewing :(
>
>
