Re: [Qemu-devel] [PATCH v1 3/3] qcow2: check for NULL l2meta

[Max Reitz <mreitz@redhat.com>]

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 <hutao@cn.fujitsu.com>
> ---
>   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 <mreitz@redhat.com>