Re: [Qemu-devel] [PATCH 4/5] qcow2: Check allocations in qcow2_check

[Kevin Wolf]

Am 26.08.2013 um 15:04 hat Max Reitz geschrieben:
> Adds a new function checking for overlapping cluster allocations.
> Furthermore, qcow2_check now marks the image as consistent if no
> corruptions have been found.
> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  block/qcow2-cluster.c | 414 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  block/qcow2.c         |  15 +-
>  block/qcow2.h         |   2 +
>  3 files changed, 429 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index be35983..ea7d334 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1499,3 +1499,417 @@ fail:
>  
>      return ret;
>  }
> +
> +static const char *const overlap_strings[8] = {
> +    "main header",
> +    "active L1 table",
> +    "active L2 table",
> +    "refcount table",
> +    "refcount block",
> +    "snapshot table",
> +    "inactive L1 table",
> +    "inactive L2 table"
> +};
> +
> +/*
> + * Checks the table and cluster allocations for inconsistencies.
> + */
> +int qcow2_check_allocations(BlockDriverState *bs, BdrvCheckResult *result,
> +                            BdrvCheckMode fix)

For the record: We have decided to leave this out as the required
functionality is already provided by the refcount checks.

> diff --git a/block/qcow2.c b/block/qcow2.c
> index 95497c6..0b0f0ac 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -311,8 +311,19 @@ static int qcow2_check(BlockDriverState *bs, 
> BdrvCheckResult *result,
>          return ret;
>      }
>  
> -    if (fix && result->check_errors == 0 && result->corruptions == 0) {
> -        return qcow2_mark_clean(bs);
> +    ret = qcow2_check_allocations(bs, result, fix);
> +    if (ret < 0) {
> +        return ret;
> +    }
> +
> +    if (result->check_errors == 0 && result->corruptions == 0) {
> +        if (fix) {
> +            ret = qcow2_mark_clean(bs);
> +            if (ret < 0) {
> +                return ret;
> +            }
> +        }
> +        return qcow2_mark_consistent(bs);

The qcow2_mark_consistent() call should probably be inside the if block,
otherwise you'll be trying to update the header of read-only image.

The reason why you didn't notice this is that qemu-img is broken since
commit 8599ea4c and doesn't report errors on negative return values any
more...

Kevin