[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v1 3/3] qcow2: check for NULL l2meta
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH v1 3/3] qcow2: check for NULL l2meta |
Date: |
Mon, 20 Jan 2014 16:17:16 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 20.01.2014 um 04:04 hat Hu Tao geschrieben:
> On Sun, Jan 19, 2014 at 05:18:05PM +0100, Max Reitz wrote:
> > On 30.12.2013 06:29, Hu Tao wrote:
> > >In case of do preallocating metadata with a large cluster size,
> > >qcow2_alloc_cluster_offset() can allocate nothing and returns
> > >a NULL l2meta. This patch checks for it and link2 l2 with only
> > >valid l2meta.
> > >
> > >Signed-off-by: Hu Tao <address@hidden>
> > >---
> > > block/qcow2.c | 14 ++++++++------
> > > 1 file changed, 8 insertions(+), 6 deletions(-)
> > >
> > >diff --git a/block/qcow2.c b/block/qcow2.c
> > >index 46860d5..380c240 100644
> > >--- a/block/qcow2.c
> > >+++ b/block/qcow2.c
> > >@@ -1399,18 +1399,20 @@ static int preallocate(BlockDriverState *bs)
> > > offset = 0;
> > > while (nb_sectors) {
> > >- num = MIN(nb_sectors, INT_MAX >> 9);
> > >+ num = MIN(nb_sectors, INT_MAX >> BDRV_SECTOR_BITS);
> >
> > Well, if you're already adjusting this here, you could also replace
> > the other occurrences of 9 and 512 in this function. ;-)
> >
> > > ret = qcow2_alloc_cluster_offset(bs, offset, &num,
> > > &host_offset, &meta);
> > > if (ret < 0) {
> > > return ret;
> > > }
> > >- ret = qcow2_alloc_cluster_link_l2(bs, meta);
> > >- if (ret < 0) {
> > >- qcow2_free_any_clusters(bs, meta->alloc_offset,
> > >meta->nb_clusters,
> > >- QCOW2_DISCARD_NEVER);
> > >- return ret;
> > >+ if (meta) {
> > >+ ret = qcow2_alloc_cluster_link_l2(bs, meta);
> > >+ if (ret < 0) {
> > >+ qcow2_free_any_clusters(bs, meta->alloc_offset,
> > >+ meta->nb_clusters,
> > >QCOW2_DISCARD_NEVER);
> > >+ return ret;
> > >+ }
> > > }
> > > /* There are no dependent requests, but we need to remove our
> > > request
> >
> > But this doesn't make this patch wrong, so:
> >
> > Reviewed-by: Max Reitz <address@hidden>
>
> Max,
>
> Thanks for all of you comments!
The series looks good in general, but I think the comments are worth
addressing before we merge it. I would also love to see an qemu-iotests
case that tests the cases that would previously crash.
Once you post a new version that addresses these points, I'll merge it.
Kevin