qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v4 1/4] qcow2: remove n_start and n_end of qcow2


From: Hu Tao
Subject: Re: [Qemu-devel] [PATCH v4 1/4] qcow2: remove n_start and n_end of qcow2_alloc_cluster_offset()
Date: Fri, 24 Jan 2014 17:32:40 +0800
User-agent: Mutt/1.5.21 (2010-09-15)

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 <address@hidden>
> > Signed-off-by: Hu Tao <address@hidden>
> > ---
> >  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().




reply via email to

[Prev in Thread] Current Thread [Next in Thread]