Re: [Qemu-devel] [PATCH 4/8] block: bdrv_img_create(): add Error ** argument

[Kevin Wolf <kwolf@redhat.com>]

Am 17.10.2012 21:35, schrieb Luiz Capitulino:
> This commit adds an Error ** argument to bdrv_img_create() and set it
> appropriately on error.
> 
> Callers of bdrv_img_create() pass NULL for the new argument and still
> rely on bdrv_img_create()'s return value. Next commits will change
> callers to use the Error object instead.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  block.c    | 23 +++++++++++++++++++++--
>  block.h    |  2 +-
>  blockdev.c |  2 +-
>  qemu-img.c |  2 +-
>  4 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/block.c b/block.c
> index bdb53af..83bcab5 100644
> --- a/block.c
> +++ b/block.c
> @@ -4295,7 +4295,7 @@ bdrv_acct_done(BlockDriverState *bs, BlockAcctCookie *cookie)
>  int bdrv_img_create(const char *filename, const char *fmt,
>                      const char *base_filename, const char *base_fmt,
>                      char *options, uint64_t img_size, int flags,
> -                    QEMUOptionParameter **param_list)
> +                    QEMUOptionParameter **param_list, Error **errp)
>  {
>      QEMUOptionParameter *param = NULL, *create_options = NULL;
>      QEMUOptionParameter *backing_fmt, *backing_file, *size;
> @@ -4308,6 +4308,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
>      drv = bdrv_find_format(fmt);
>      if (!drv) {
>          error_report("Unknown file format '%s'", fmt);
> +        error_setg(errp, "Unknown file format '%s'", fmt);
>          ret = -EINVAL;
>          goto out;
>      }
> @@ -4315,6 +4316,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
>      proto_drv = bdrv_find_protocol(filename);
>      if (!proto_drv) {
>          error_report("Unknown protocol '%s'", filename);
> +        error_setg(errp, "Unknown protocol '%s'", filename);
>          ret = -EINVAL;
>          goto out;
>      }
> @@ -4334,6 +4336,7 @@ int bdrv_img_create(const char *filename, const char *fmt,
>          param = parse_option_parameters(options, create_options, param);
>          if (param == NULL) {
>              error_report("Invalid options for file format '%s'.", fmt);
> +            error_setg(errp, "Invalid options for file format '%s'.", fmt);
>              ret = -EINVAL;
>              goto out;
>          }
> @@ -4344,6 +4347,8 @@ int bdrv_img_create(const char *filename, const char *fmt,
>                                   base_filename)) {
>              error_report("Backing file not supported for file format '%s'",
>                           fmt);
> +            error_setg(errp, "Backing file not supported for file format '%s'",
> +                       fmt);
>              ret = -EINVAL;
>              goto out;
>          }
> @@ -4353,6 +4358,8 @@ int bdrv_img_create(const char *filename, const char *fmt,
>          if (set_option_parameter(param, BLOCK_OPT_BACKING_FMT, base_fmt)) {
>              error_report("Backing file format not supported for file "
>                           "format '%s'", fmt);
> +            error_setg(errp, "Backing file format not supported for file "
> +                             "format '%s'", fmt);
>              ret = -EINVAL;
>              goto out;
>          }
> @@ -4363,6 +4370,8 @@ int bdrv_img_create(const char *filename, const char *fmt,
>          if (!strcmp(filename, backing_file->value.s)) {
>              error_report("Error: Trying to create an image with the "
>                           "same filename as the backing file");
> +            error_setg(errp, "Error: Trying to create an image with the "
> +                             "same filename as the backing file");
>              ret = -EINVAL;
>              goto out;
>          }
> @@ -4374,6 +4383,8 @@ int bdrv_img_create(const char *filename, const char *fmt,
>          if (!backing_drv) {
>              error_report("Unknown backing file format '%s'",
>                           backing_fmt->value.s);
> +            error_setg(errp, "Unknown backing file format '%s'",
> +                       backing_fmt->value.s);
>              ret = -EINVAL;
>              goto out;
>          }
> @@ -4396,7 +4407,8 @@ int bdrv_img_create(const char *filename, const char *fmt,
>  
>              ret = bdrv_open(bs, backing_file->value.s, back_flags, backing_drv);
>              if (ret < 0) {
> -                error_report("Could not open '%s'", backing_file->value.s);

This is the only error_report() that is dropped in this patch. Not sure
what is the best patch to drop them, but consistency wouldn't hurt.

If we want to keep the series bisectable, i.e. qemu-img output stays the
same for each patch, you'd have to drop them in 5/8, I think.

Kevin