qemu-devel
[Top][All Lists]
Advanced

[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: Hu Tao
Subject: Re: [Qemu-devel] [PATCH v1 3/3] qcow2: check for NULL l2meta
Date: Mon, 20 Jan 2014 11:04:27 +0800
User-agent: Mutt/1.5.21 (2010-09-15)

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!



reply via email to

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