diff mbox

[v4,3/4] qcow2: check for NULL l2meta

Message ID d5b8b878c74af869b4e2d0a6c32118a1e86e16ea.1390445921.git.hutao@cn.fujitsu.com
State New
Headers show

Commit Message

Hu Tao Jan. 23, 2014, 3:04 a.m. UTC
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.

Replace 9 and 512 with BDRV_SECTOR_BITS, BDRV_SECTOR_SIZE
respectively while at the function.

Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 block/qcow2.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

Comments

Benoît Canet Jan. 23, 2014, 5:20 p.m. UTC | #1
Le Thursday 23 Jan 2014 à 11:04:07 (+0800), Hu Tao a écrit :
> In case of do preallocating metadata with a large cluster size,
In the case of a metadata preallocation with

> qcow2_alloc_cluster_offset() can allocate nothing and returns
> a NULL l2meta. This patch checks for it and link2 l2 with only
> valid l2meta.
> 
> Replace 9 and 512 with BDRV_SECTOR_BITS, BDRV_SECTOR_SIZE
> respectively while at the function.
> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  block/qcow2.c | 25 ++++++++++++++-----------
>  1 file changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 0a310cc..f989247 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1390,22 +1390,24 @@ static int preallocate(BlockDriverState *bs)
>      int ret;
>      QCowL2Meta *meta;
>  
> -    nb_sectors = bdrv_getlength(bs) >> 9;
> +    nb_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
>      offset = 0;
>  
>      while (nb_sectors) {
> -        num = MIN(nb_sectors, INT_MAX >> 9);
> +        num = MIN(nb_sectors, INT_MAX >> BDRV_SECTOR_BITS);
>          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;
> +            }
>          }

Maybe if (meta) could include the following line to remove another extra test.
QLIST_REMOVE(meta, next_in_flight);

>  
>          /* There are no dependent requests, but we need to remove our request
> @@ -1417,7 +1419,7 @@ static int preallocate(BlockDriverState *bs)
>          /* TODO Preallocate data if requested */
>  
>          nb_sectors -= num;
> -        offset += num << 9;
> +        offset += num << BDRV_SECTOR_BITS;
>      }
>  
>      /*
> @@ -1426,9 +1428,10 @@ static int preallocate(BlockDriverState *bs)
>       * EOF). Extend the image to the last allocated sector.
>       */
>      if (host_offset != 0) {
> -        uint8_t buf[512];
> -        memset(buf, 0, 512);
> -        ret = bdrv_write(bs->file, (host_offset >> 9) + num - 1, buf, 1);
> +        uint8_t buf[BDRV_SECTOR_SIZE];
> +        memset(buf, 0, BDRV_SECTOR_SIZE);
> +        ret = bdrv_write(bs->file, (host_offset >> BDRV_SECTOR_BITS) + num - 1,
> +                         buf, 1);
>          if (ret < 0) {
>              return ret;
>          }
> -- 
> 1.8.5.2.229.g4448466
> 
>
diff mbox

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index 0a310cc..f989247 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1390,22 +1390,24 @@  static int preallocate(BlockDriverState *bs)
     int ret;
     QCowL2Meta *meta;
 
-    nb_sectors = bdrv_getlength(bs) >> 9;
+    nb_sectors = bdrv_getlength(bs) >> BDRV_SECTOR_BITS;
     offset = 0;
 
     while (nb_sectors) {
-        num = MIN(nb_sectors, INT_MAX >> 9);
+        num = MIN(nb_sectors, INT_MAX >> BDRV_SECTOR_BITS);
         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
@@ -1417,7 +1419,7 @@  static int preallocate(BlockDriverState *bs)
         /* TODO Preallocate data if requested */
 
         nb_sectors -= num;
-        offset += num << 9;
+        offset += num << BDRV_SECTOR_BITS;
     }
 
     /*
@@ -1426,9 +1428,10 @@  static int preallocate(BlockDriverState *bs)
      * EOF). Extend the image to the last allocated sector.
      */
     if (host_offset != 0) {
-        uint8_t buf[512];
-        memset(buf, 0, 512);
-        ret = bdrv_write(bs->file, (host_offset >> 9) + num - 1, buf, 1);
+        uint8_t buf[BDRV_SECTOR_SIZE];
+        memset(buf, 0, BDRV_SECTOR_SIZE);
+        ret = bdrv_write(bs->file, (host_offset >> BDRV_SECTOR_BITS) + num - 1,
+                         buf, 1);
         if (ret < 0) {
             return ret;
         }