If we want to add some info to errp (by error_prepend() or
error_append_hint()), we must use the ERRP_AUTO_PROPAGATE macro.
Otherwise, this info will not be added when errp == &fatal_err
(the program will exit prior to the error_append_hint() or
error_prepend() call). Fix such cases.
If we want to check error after errp-function call, we need to
introduce local_err and than propagate it to errp. Instead, use
ERRP_AUTO_PROPAGATE macro, benefits are:
1. No need of explicit error_propagate call
2. No need of explicit local_err variable: use errp directly
3. ERRP_AUTO_PROPAGATE leaves errp as is if it's not NULL or
&error_fatel, this means that we don't break error_abort
(we'll abort on error_set, not on error_propagate)
This commit (together with its neighbors) was generated by
for f in $(git grep -l errp \*.[ch]); do \
spatch --sp-file scripts/coccinelle/auto-propagated-errp.cocci \
--macro-file scripts/cocci-macro-file.h --in-place --no-show-diff $f; \
done;
then fix a bit of compilation problems: coccinelle for some reason
leaves several
f() {
...
goto out;
...
out:
}
patterns, with "out:" at function end.
then
./python/commit-per-subsystem.py MAINTAINERS "$(< auto-msg)"
(auto-msg was a file with this commit message)
Still, for backporting it may be more comfortable to use only the first
command and then do one huge commit.
Reported-by: Kevin Wolf <kw...@redhat.com>
Reported-by: Greg Kurz <gr...@kaod.org>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
---
block/replication.c | 40 ++++++++++++++++++----------------------
replication.c | 28 ++++++++++++----------------
2 files changed, 30 insertions(+), 38 deletions(-)
diff --git a/block/replication.c b/block/replication.c
index 936b2f8b5a..f5e7c5a8ec 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -83,9 +83,9 @@ static ReplicationOps replication_ops = {
static int replication_open(BlockDriverState *bs, QDict *options,
int flags, Error **errp)
{
+ ERRP_AUTO_PROPAGATE();
int ret;
BDRVReplicationState *s = bs->opaque;
- Error *local_err = NULL;
QemuOpts *opts = NULL;
const char *mode;
const char *top_id;
@@ -98,14 +98,14 @@ static int replication_open(BlockDriverState *bs, QDict
*options,
ret = -EINVAL;
opts = qemu_opts_create(&replication_runtime_opts, NULL, 0, &error_abort);
- qemu_opts_absorb_qdict(opts, options, &local_err);
- if (local_err) {
+ qemu_opts_absorb_qdict(opts, options, errp);
+ if (*errp) {
goto fail;
}
mode = qemu_opt_get(opts, REPLICATION_MODE);
if (!mode) {
- error_setg(&local_err, "Missing the option mode");
+ error_setg(errp, "Missing the option mode");
goto fail;
}
@@ -113,7 +113,8 @@ static int replication_open(BlockDriverState *bs, QDict
*options,
s->mode = REPLICATION_MODE_PRIMARY;
top_id = qemu_opt_get(opts, REPLICATION_TOP_ID);
if (top_id) {
- error_setg(&local_err, "The primary side does not support option
top-id");
+ error_setg(errp,
+ "The primary side does not support option top-id");
goto fail;
}
} else if (!strcmp(mode, "secondary")) {
@@ -121,11 +122,11 @@ static int replication_open(BlockDriverState *bs, QDict
*options,
top_id = qemu_opt_get(opts, REPLICATION_TOP_ID);
s->top_id = g_strdup(top_id);
if (!s->top_id) {
- error_setg(&local_err, "Missing the option top-id");
+ error_setg(errp, "Missing the option top-id");
goto fail;
}
} else {
- error_setg(&local_err,
+ error_setg(errp,
"The option mode's value should be primary or secondary");
goto fail;
}
@@ -136,7 +137,6 @@ static int replication_open(BlockDriverState *bs, QDict
*options,
fail:
qemu_opts_del(opts);
- error_propagate(errp, local_err);
return ret;
}
@@ -314,7 +314,7 @@ static bool
replication_recurse_is_first_non_filter(BlockDriverState *bs,
static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)
{
- Error *local_err = NULL;
+ ERRP_AUTO_PROPAGATE();
int ret;
if (!s->backup_job) {
@@ -322,9 +322,8 @@ static void secondary_do_checkpoint(BDRVReplicationState
*s, Error **errp)
return;
}
- backup_do_checkpoint(s->backup_job, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ backup_do_checkpoint(s->backup_job, errp);
+ if (*errp) {
return;
}
@@ -361,9 +360,9 @@ static void secondary_do_checkpoint(BDRVReplicationState
*s, Error **errp)
static void reopen_backing_file(BlockDriverState *bs, bool writable,
Error **errp)
{
+ ERRP_AUTO_PROPAGATE();
BDRVReplicationState *s = bs->opaque;
BlockReopenQueue *reopen_queue = NULL;
- Error *local_err = NULL;
if (writable) {
s->orig_hidden_read_only = bdrv_is_read_only(s->hidden_disk->bs);
@@ -388,8 +387,7 @@ static void reopen_backing_file(BlockDriverState *bs, bool
writable,
}
if (reopen_queue) {
- bdrv_reopen_multiple(reopen_queue, &local_err);
- error_propagate(errp, local_err);
+ bdrv_reopen_multiple(reopen_queue, errp);
}
bdrv_subtree_drained_end(s->hidden_disk->bs);
@@ -445,12 +443,12 @@ static bool check_top_bs(BlockDriverState *top_bs,
BlockDriverState *bs)
static void replication_start(ReplicationState *rs, ReplicationMode mode,
Error **errp)
{
+ ERRP_AUTO_PROPAGATE();
BlockDriverState *bs = rs->opaque;
BDRVReplicationState *s;
BlockDriverState *top_bs;
int64_t active_length, hidden_length, disk_length;
AioContext *aio_context;
- Error *local_err = NULL;
aio_context = bdrv_get_aio_context(bs);
aio_context_acquire(aio_context);
@@ -519,9 +517,8 @@ static void replication_start(ReplicationState *rs,
ReplicationMode mode,
}
/* reopen the backing file in r/w mode */
- reopen_backing_file(bs, true, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ reopen_backing_file(bs, true, errp);
+ if (*errp) {
aio_context_release(aio_context);
return;
}
@@ -546,9 +543,8 @@ static void replication_start(ReplicationState *rs,
ReplicationMode mode,
0, MIRROR_SYNC_MODE_NONE, NULL, 0, false,
BLOCKDEV_ON_ERROR_REPORT,
BLOCKDEV_ON_ERROR_REPORT, JOB_INTERNAL,
- backup_job_completed, bs, NULL, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ backup_job_completed, bs, NULL, errp);
+ if (*errp) {
backup_job_cleanup(bs);
aio_context_release(aio_context);
return;
diff --git a/replication.c b/replication.c
index be3a42f9c9..2e5ea7f537 100644
--- a/replication.c
+++ b/replication.c
@@ -44,15 +44,14 @@ void replication_remove(ReplicationState *rs)
*/
void replication_start_all(ReplicationMode mode, Error **errp)
{
+ ERRP_AUTO_PROPAGATE();
ReplicationState *rs, *next;
- Error *local_err = NULL;
QLIST_FOREACH_SAFE(rs, &replication_states, node, next) {
if (rs->ops && rs->ops->start) {
- rs->ops->start(rs, mode, &local_err);
+ rs->ops->start(rs, mode, errp);
}
- if (local_err) {
- error_propagate(errp, local_err);
+ if (*errp) {
return;
}
}
@@ -60,15 +59,14 @@ void replication_start_all(ReplicationMode mode, Error
**errp)
void replication_do_checkpoint_all(Error **errp)
{
+ ERRP_AUTO_PROPAGATE();
ReplicationState *rs, *next;
- Error *local_err = NULL;
QLIST_FOREACH_SAFE(rs, &replication_states, node, next) {
if (rs->ops && rs->ops->checkpoint) {
- rs->ops->checkpoint(rs, &local_err);
+ rs->ops->checkpoint(rs, errp);
}
- if (local_err) {
- error_propagate(errp, local_err);
+ if (*errp) {
return;
}
}
@@ -76,15 +74,14 @@ void replication_do_checkpoint_all(Error **errp)
void replication_get_error_all(Error **errp)
{
+ ERRP_AUTO_PROPAGATE();
ReplicationState *rs, *next;
- Error *local_err = NULL;
QLIST_FOREACH_SAFE(rs, &replication_states, node, next) {
if (rs->ops && rs->ops->get_error) {
- rs->ops->get_error(rs, &local_err);
+ rs->ops->get_error(rs, errp);
}
- if (local_err) {
- error_propagate(errp, local_err);
+ if (*errp) {
return;
}
}
@@ -92,15 +89,14 @@ void replication_get_error_all(Error **errp)
void replication_stop_all(bool failover, Error **errp)
{
+ ERRP_AUTO_PROPAGATE();
ReplicationState *rs, *next;
- Error *local_err = NULL;
QLIST_FOREACH_SAFE(rs, &replication_states, node, next) {
if (rs->ops && rs->ops->stop) {
- rs->ops->stop(rs, failover, &local_err);
+ rs->ops->stop(rs, failover, errp);
}
- if (local_err) {
- error_propagate(errp, local_err);
+ if (*errp) {
return;
}
}
--
2.21.0
