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 <mre...@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
