Re: [Qemu-devel] [RFC v2 6/6] bdrv: No silent error message discarding

[Kevin Wolf <kwolf@redhat.com>]

Am 05.09.2013 um 15:55 hat Max Reitz geschrieben:
> Using NULL as errp parameters to bdrv_open, bdrv_file_open and
> bdrv_create in all block drivers which do not yet implement proper error
> propagation is not a good idea, since this now silently discards error
> messages. Fix this by using a proper parameter and printing its content
> on error (until proper propagation is implemented).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> Note that many error propagation are actually trivial (such as in
> raw_bsd), but I intentionally refrained from implementing the propagation
> and tenaciously followed the pattern:
>   Error *local_err = NULL;
>   foo(&local_err);
>   if (/*error condition*/) {
>       qerror_report_err(local_err):
>       error_free(local_err);
>   }
> This is because the propagation may sometimes be trivial, however, it is
> often still driver-specific, therefore this deserves its own patch for
> every driver, in my opinion. Also, I think it is easier to review this
> way. ;-)

Agreed. But this patch should be squashed into the patches introducing
the NULL argument. Otherwise I'd need to check if this patch catches all
places that were previously introduced.

> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 6722771..a9d8019 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -1401,9 +1401,10 @@ static int sd_prealloc(const char *filename)
>      uint32_t idx, max_idx;
>      int64_t vdi_size;
>      void *buf = g_malloc0(SD_DATA_OBJ_SIZE);
> +    Error *local_err = NULL;
>      int ret;
>  
> -    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, NULL);
> +    ret = bdrv_file_open(&bs, filename, NULL, BDRV_O_RDWR, &local_err);
>      if (ret < 0) {
>          goto out;
>      }

You didn't feel like printing the message here? :-)

Kevin