Re: [Qemu-devel] [RFC v2 5/6] qcow2: Use Error parameter

[Kevin Wolf <kwolf@redhat.com>]

Am 05.09.2013 um 15:55 hat Max Reitz geschrieben:
> Employ usage of the new Error ** parameter in qcow2_open, qcow2_create
> and associated functions.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 135 ++++++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 88 insertions(+), 47 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index e16d352..9a96188 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1,4 +1,5 @@
>  /*
> + * error_setg(errp, "
>   * Block driver for the QCOW version 2 format
>   *
>   * Copyright (c) 2004-2006 Fabrice Bellard

Uhm... No? :-)

> @@ -79,7 +80,8 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
>   * return 0 upon success, non-0 otherwise
>   */
>  static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
> -                                 uint64_t end_offset, void **p_feature_table)
> +                                 uint64_t end_offset, void **p_feature_table,
> +                                 Error **errp)
>  {
>      BDRVQcowState *s = bs->opaque;
>      QCowExtension ext;
> @@ -100,10 +102,10 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
>          printf("attempting to read extended header in offset %lu\n", offset);
>  #endif
>  
> -        if (bdrv_pread(bs->file, offset, &ext, sizeof(ext)) != sizeof(ext)) {
> -            fprintf(stderr, "qcow2_read_extension: ERROR: "
> -                    "pread fail from offset %" PRIu64 "\n",
> -                    offset);
> +        ret = bdrv_pread(bs->file, offset, &ext, sizeof(ext));
> +        if (ret < 0) {
> +            error_setg_errno(errp, -ret, "qcow2_read_extension: ERROR: "
> +                             "pread fail from offset %" PRIu64, offset);
>              return 1;
>          }
>          be32_to_cpus(&ext.magic);
> @@ -113,7 +115,7 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
>          printf("ext.magic = 0x%x\n", ext.magic);
>  #endif
>          if (ext.len > end_offset - offset) {
> -            error_report("Header extension too large");
> +            error_setg(errp, "Header extension too large");
>              return -EINVAL;
>          }
>  
> @@ -123,17 +125,19 @@ static int qcow2_read_extensions(BlockDriverState *bs, uint64_t start_offset,
>  
>          case QCOW2_EXT_MAGIC_BACKING_FORMAT:
>              if (ext.len >= sizeof(bs->backing_format)) {
> -                fprintf(stderr, "ERROR: ext_backing_format: len=%u too large"
> -                        " (>=%zu)\n",
> -                        ext.len, sizeof(bs->backing_format));
> +                error_setg(errp, "ERROR: ext_backing_format: len=%u too large"
> +                           " (>=%zu)", ext.len, sizeof(bs->backing_format));
>                  return 2;
>              }
> -            if (bdrv_pread(bs->file, offset , bs->backing_format,
> -                           ext.len) != ext.len)
> +            ret = bdrv_pread(bs->file, offset, bs->backing_format, ext.len);
> +            if (ret < 0) {
> +                error_setg_errno(errp, -ret, "ERROR: ext_backing_format: "
> +                                 "Could not read format name");
>                  return 3;
> +            }
>              bs->backing_format[ext.len] = '\0';
>  #ifdef DEBUG_EXT
> -            printf("Qcow2: Got format extension %s\n", bs->backing_format);
> +            printf("Qcow2: Got format extension %s", bs->backing_format);

Huh?

>  #endif
>              break;
>  

> @@ -1412,8 +1442,9 @@ static int qcow2_create2(const char *filename, int64_t total_size,
>      BlockDriver* drv = bdrv_find_format("qcow2");
>      assert(drv != NULL);
>      ret = bdrv_open(bs, filename, NULL,
> -        BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, NULL);
> +        BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH, drv, &local_err);
>      if (ret < 0) {
> +        error_propagate(errp, local_err);
>          goto out;
>      }

More context:

    ret = qcow2_alloc_clusters(bs, 2 * cluster_size);
    if (ret < 0) {
        goto out;

Missing error_setg_errno()?

Kevin