17.09.2019 13:20, Greg Kurz wrote:
> Ensure that hints are added even if errp is &error_fatal or &error_abort.
>
> Signed-off-by: Greg Kurz <gr...@kaod.org>
> ---
> block/backup.c | 7 +++++--
> block/dirty-bitmap.c | 7 +++++--
> block/file-posix.c | 20 +++++++++++++-------
> block/gluster.c | 23 +++++++++++++++--------
> block/qcow.c | 10 ++++++----
> block/qcow2.c | 7 +++++--
> block/vhdx-log.c | 7 +++++--
> block/vpc.c | 7 +++++--
> 8 files changed, 59 insertions(+), 29 deletions(-)
>
> diff --git a/block/backup.c b/block/backup.c
> index 763f0d7ff6db..d8c422a0e3bc 100644
> --- a/block/backup.c
> +++ b/block/backup.c
> @@ -602,11 +602,14 @@ static int64_t
> backup_calculate_cluster_size(BlockDriverState *target,
> BACKUP_CLUSTER_SIZE_DEFAULT);
> return BACKUP_CLUSTER_SIZE_DEFAULT;
> } else if (ret < 0 && !target->backing) {
> - error_setg_errno(errp, -ret,
> + Error *local_err = NULL;
> +
> + error_setg_errno(&local_err, -ret,
> "Couldn't determine the cluster size of the target image, "
> "which has no backing file");
> - error_append_hint(errp,
> + error_append_hint(&local_err,
> "Aborting, since this may create an unusable destination
> image\n");
> + error_propagate(errp, local_err);
> return ret;
> } else if (ret < 0 && target->backing) {
> /* Not fatal; just trudge on ahead. */
Pain.. Do we need these hints, if they are so painful?
At least for cases like this, we can create helper function
error_setg_errno_hint(..., error, hint)
But what could be done when we call function, which may or may not set errp?
ret = f(errp);
if (ret) {
error_append_hint(errp, hint);
}
Hmmm..
Can it look like
ret = f(..., hint_helper(errp, hint))
?
I can't imagine how to do it, as someone should remove hint from error_abort
object on
success path..
But seems, the following is possible, which seems better for me than
local-error approach:
error_push_hint(errp, hint);
ret = f(.., errp);
error_pop_hint(errp);
===
Continue thinking on this:
It may look like just
ret = f(..., set_hint(errp, hint));
or (just to split long line):
set_hint(errp, hint);
ret = f(..., errp);
if in each function with errp does error_push_hint(errp) on start and
error_pop_hint(errp) on exit,
which may be just one call at function start of macro, which will call
error_push_hint(errp) and
define local variable by g_auto, with cleanup which will call
error_pop_hint(errp) on function
exit..
Or, may be, more direct way to set cleanup for function exists?
===
Also, we can implement some code generation, to generate for functions with
errp argument
wrappers with additional hint parameter, and just use these wrappers..
===
If nobody likes any of my suggestions, then ignore them. I understand that this
series fixes
real issue and much effort has already been spent. May be one day I'll try to
rewrite it...
--
Best regards,
Vladimir
