From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-9.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AAE97C47404 for ; Fri, 11 Oct 2019 18:32:15 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 6E137206CD for ; Fri, 11 Oct 2019 18:32:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6E137206CD Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=virtuozzo.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:55530 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iIzi2-0005wB-Bw for qemu-devel@archiver.kernel.org; Fri, 11 Oct 2019 14:32:14 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:40615) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iIxhK-00015v-De for qemu-devel@nongnu.org; Fri, 11 Oct 2019 12:23:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iIxhI-0008NX-CM for qemu-devel@nongnu.org; Fri, 11 Oct 2019 12:23:22 -0400 Received: from relay.sw.ru ([185.231.240.75]:49704) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1iIxhI-0008N4-0Z; Fri, 11 Oct 2019 12:23:20 -0400 Received: from [10.94.3.0] (helo=kvm.qa.sw.ru) by relay.sw.ru with esmtp (Exim 4.92.2) (envelope-from ) id 1iIxR6-0003XG-Nk; Fri, 11 Oct 2019 19:06:37 +0300 From: Vladimir Sementsov-Ogievskiy To: qemu-devel@nongnu.org Subject: [RFC v5 111/126] raw: introduce ERRP_AUTO_PROPAGATE Date: Fri, 11 Oct 2019 19:05:37 +0300 Message-Id: <20191011160552.22907-112-vsementsov@virtuozzo.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20191011160552.22907-1-vsementsov@virtuozzo.com> References: <20191011160552.22907-1-vsementsov@virtuozzo.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 185.231.240.75 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , vsementsov@virtuozzo.com, qemu-block@nongnu.org, Stefan Weil , armbru@redhat.com, Max Reitz , Greg Kurz Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" 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 Reported-by: Greg Kurz Signed-off-by: Vladimir Sementsov-Ogievskiy --- block/file-posix.c | 79 ++++++++++++++++++++-------------------------- block/file-win32.c | 29 +++++++---------- block/raw-format.c | 7 ++-- 3 files changed, 50 insertions(+), 65 deletions(-) diff --git a/block/file-posix.c b/block/file-posix.c index f12c06de2d..fa75232713 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -320,6 +320,7 @@ static bool raw_is_io_aligned(int fd, void *buf, size_t len) static void raw_probe_alignment(BlockDriverState *bs, int fd, Error **errp) { + ERRP_AUTO_PROPAGATE(); BDRVRawState *s = bs->opaque; char *buf; size_t max_align = MAX(MAX_BLOCKSIZE, getpagesize()); @@ -473,9 +474,9 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, int bdrv_flags, int open_flags, bool device, Error **errp) { + ERRP_AUTO_PROPAGATE(); BDRVRawState *s = bs->opaque; QemuOpts *opts; - Error *local_err = NULL; const char *filename = NULL; const char *str; BlockdevAioOptions aio, aio_default; @@ -484,9 +485,8 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, OnOffAuto locking; opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort); - qemu_opts_absorb_qdict(opts, options, &local_err); - if (local_err) { - error_propagate(errp, local_err); + qemu_opts_absorb_qdict(opts, options, errp); + if (*errp) { ret = -EINVAL; goto fail; } @@ -503,9 +503,8 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, : BLOCKDEV_AIO_OPTIONS_THREADS; aio = qapi_enum_parse(&BlockdevAioOptions_lookup, qemu_opt_get(opts, "aio"), - aio_default, &local_err); - if (local_err) { - error_propagate(errp, local_err); + aio_default, errp); + if (*errp) { ret = -EINVAL; goto fail; } @@ -513,9 +512,8 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, locking = qapi_enum_parse(&OnOffAuto_lookup, qemu_opt_get(opts, "locking"), - ON_OFF_AUTO_AUTO, &local_err); - if (local_err) { - error_propagate(errp, local_err); + ON_OFF_AUTO_AUTO, errp); + if (*errp) { ret = -EINVAL; goto fail; } @@ -541,9 +539,8 @@ static int raw_open_common(BlockDriverState *bs, QDict *options, str = qemu_opt_get(opts, "pr-manager"); if (str) { - s->pr_mgr = pr_manager_lookup(str, &local_err); - if (local_err) { - error_propagate(errp, local_err); + s->pr_mgr = pr_manager_lookup(str, errp); + if (*errp) { ret = -EINVAL; goto fail; } @@ -817,9 +814,9 @@ static int raw_handle_perm_lock(BlockDriverState *bs, uint64_t new_perm, uint64_t new_shared, Error **errp) { + ERRP_AUTO_PROPAGATE(); BDRVRawState *s = bs->opaque; int ret = 0; - Error *local_err = NULL; if (!s->use_lock) { return 0; @@ -859,22 +856,22 @@ static int raw_handle_perm_lock(BlockDriverState *bs, /* fall through to unlock bytes. */ case RAW_PL_ABORT: raw_apply_lock_bytes(s, s->fd, s->perm, ~s->shared_perm, - true, &local_err); - if (local_err) { + true, errp); + if (*errp) { /* Theoretically the above call only unlocks bytes and it cannot * fail. Something weird happened, report it. */ - warn_report_err(local_err); + warn_report_errp(errp); } break; case RAW_PL_COMMIT: raw_apply_lock_bytes(s, s->fd, new_perm, ~new_shared, - true, &local_err); - if (local_err) { + true, errp); + if (*errp) { /* Theoretically the above call only unlocks bytes and it cannot * fail. Something weird happened, report it. */ - warn_report_err(local_err); + warn_report_errp(errp); } break; } @@ -948,11 +945,11 @@ static int raw_reconfigure_getfd(BlockDriverState *bs, int flags, static int raw_reopen_prepare(BDRVReopenState *state, BlockReopenQueue *queue, Error **errp) { + ERRP_AUTO_PROPAGATE(); BDRVRawState *s; BDRVRawReopenState *rs; QemuOpts *opts; int ret; - Error *local_err = NULL; assert(state != NULL); assert(state->bs != NULL); @@ -964,9 +961,8 @@ static int raw_reopen_prepare(BDRVReopenState *state, /* Handle options changes */ opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort); - qemu_opts_absorb_qdict(opts, state->options, &local_err); - if (local_err) { - error_propagate(errp, local_err); + qemu_opts_absorb_qdict(opts, state->options, errp); + if (*errp) { ret = -EINVAL; goto out; } @@ -981,9 +977,8 @@ static int raw_reopen_prepare(BDRVReopenState *state, qemu_opts_to_qdict(opts, state->options); rs->fd = raw_reconfigure_getfd(state->bs, state->flags, &rs->open_flags, - state->perm, true, &local_err); - if (local_err) { - error_propagate(errp, local_err); + state->perm, true, errp); + if (*errp) { ret = -1; goto out; } @@ -991,9 +986,8 @@ static int raw_reopen_prepare(BDRVReopenState *state, /* Fail already reopen_prepare() if we can't get a working O_DIRECT * alignment with the new fd. */ if (rs->fd != -1) { - raw_probe_alignment(state->bs, rs->fd, &local_err); - if (local_err) { - error_propagate(errp, local_err); + raw_probe_alignment(state->bs, rs->fd, errp); + if (*errp) { ret = -EINVAL; goto out_fd; } @@ -2232,8 +2226,8 @@ static int64_t raw_get_allocated_file_size(BlockDriverState *bs) static int coroutine_fn raw_co_create(BlockdevCreateOptions *options, Error **errp) { + ERRP_AUTO_PROPAGATE(); BlockdevCreateOptionsFile *file_opts; - Error *local_err = NULL; int fd; uint64_t perm, shared; int result = 0; @@ -2315,13 +2309,13 @@ raw_co_create(BlockdevCreateOptions *options, Error **errp) } out_unlock: - raw_apply_lock_bytes(NULL, fd, 0, 0, true, &local_err); - if (local_err) { + raw_apply_lock_bytes(NULL, fd, 0, 0, true, errp); + if (*errp) { /* The above call should not fail, and if it does, that does * not mean the whole creation operation has failed. So * report it the user for their convenience, but do not report * it to the caller. */ - warn_report_err(local_err); + warn_report_errp(errp); } out_close: @@ -2336,12 +2330,12 @@ out: static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts, Error **errp) { + ERRP_AUTO_PROPAGATE(); BlockdevCreateOptions options; int64_t total_size = 0; bool nocow = false; PreallocMode prealloc; char *buf = NULL; - Error *local_err = NULL; /* Skip file: protocol prefix */ strstart(filename, "file:", &filename); @@ -2352,10 +2346,9 @@ static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts, nocow = qemu_opt_get_bool(opts, BLOCK_OPT_NOCOW, false); buf = qemu_opt_get_del(opts, BLOCK_OPT_PREALLOC); prealloc = qapi_enum_parse(&PreallocMode_lookup, buf, - PREALLOC_MODE_OFF, &local_err); + PREALLOC_MODE_OFF, errp); g_free(buf); - if (local_err) { - error_propagate(errp, local_err); + if (*errp) { return -EINVAL; } @@ -3155,8 +3148,8 @@ static bool hdev_is_sg(BlockDriverState *bs) static int hdev_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { + ERRP_AUTO_PROPAGATE(); BDRVRawState *s = bs->opaque; - Error *local_err = NULL; int ret; #if defined(__APPLE__) && defined(__MACH__) @@ -3221,9 +3214,8 @@ hdev_open_Mac_error: s->type = FTYPE_FILE; - ret = raw_open_common(bs, options, flags, 0, true, &local_err); + ret = raw_open_common(bs, options, flags, 0, true, errp); if (ret < 0) { - error_propagate(errp, local_err); #if defined(__APPLE__) && defined(__MACH__) if (*bsd_path) { filename = bsd_path; @@ -3557,15 +3549,14 @@ static BlockDriver bdrv_host_cdrom = { static int cdrom_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { + ERRP_AUTO_PROPAGATE(); BDRVRawState *s = bs->opaque; - Error *local_err = NULL; int ret; s->type = FTYPE_CD; - ret = raw_open_common(bs, options, flags, 0, true, &local_err); + ret = raw_open_common(bs, options, flags, 0, true, errp); if (ret) { - error_propagate(errp, local_err); return ret; } diff --git a/block/file-win32.c b/block/file-win32.c index 41f55dfece..6ef3117cda 100644 --- a/block/file-win32.c +++ b/block/file-win32.c @@ -326,11 +326,11 @@ static bool get_aio_option(QemuOpts *opts, int flags, Error **errp) static int raw_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { + ERRP_AUTO_PROPAGATE(); BDRVRawState *s = bs->opaque; int access_flags; DWORD overlapped; QemuOpts *opts; - Error *local_err = NULL; const char *filename; bool use_aio; int ret; @@ -338,9 +338,8 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, s->type = FTYPE_FILE; opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort); - qemu_opts_absorb_qdict(opts, options, &local_err); - if (local_err) { - error_propagate(errp, local_err); + qemu_opts_absorb_qdict(opts, options, errp); + if (*errp) { ret = -EINVAL; goto fail; } @@ -353,9 +352,8 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, filename = qemu_opt_get(opts, "filename"); - use_aio = get_aio_option(opts, flags, &local_err); - if (local_err) { - error_propagate(errp, local_err); + use_aio = get_aio_option(opts, flags, errp); + if (*errp) { ret = -EINVAL; goto fail; } @@ -722,33 +720,30 @@ static void hdev_refresh_limits(BlockDriverState *bs, Error **errp) static int hdev_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { + ERRP_AUTO_PROPAGATE(); BDRVRawState *s = bs->opaque; int access_flags, create_flags; int ret = 0; DWORD overlapped; char device_name[64]; - - Error *local_err = NULL; const char *filename; bool use_aio; QemuOpts *opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort); - qemu_opts_absorb_qdict(opts, options, &local_err); - if (local_err) { - error_propagate(errp, local_err); + qemu_opts_absorb_qdict(opts, options, errp); + if (*errp) { ret = -EINVAL; goto done; } filename = qemu_opt_get(opts, "filename"); - use_aio = get_aio_option(opts, flags, &local_err); - if (!local_err && use_aio) { - error_setg(&local_err, "AIO is not supported on Windows host devices"); + use_aio = get_aio_option(opts, flags, errp); + if (!*errp && use_aio) { + error_setg(errp, "AIO is not supported on Windows host devices"); } - if (local_err) { - error_propagate(errp, local_err); + if (*errp) { ret = -EINVAL; goto done; } diff --git a/block/raw-format.c b/block/raw-format.c index 42c28cc29a..8903b54499 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -74,7 +74,7 @@ static QemuOptsList raw_create_opts = { static int raw_read_options(QDict *options, BlockDriverState *bs, BDRVRawState *s, Error **errp) { - Error *local_err = NULL; + ERRP_AUTO_PROPAGATE(); QemuOpts *opts = NULL; int64_t real_size = 0; int ret; @@ -86,9 +86,8 @@ static int raw_read_options(QDict *options, BlockDriverState *bs, } opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort); - qemu_opts_absorb_qdict(opts, options, &local_err); - if (local_err) { - error_propagate(errp, local_err); + qemu_opts_absorb_qdict(opts, options, errp); + if (*errp) { ret = -EINVAL; goto end; } -- 2.21.0