From patchwork Mon May 9 12:07:50 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 9046201 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id BC92CBF29F for ; Mon, 9 May 2016 12:40:59 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C8F3720114 for ; Mon, 9 May 2016 12:40:58 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 9D0652010B for ; Mon, 9 May 2016 12:40:57 +0000 (UTC) Received: from localhost ([::1]:41060 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1azkUa-0000tK-TW for patchwork-qemu-devel@patchwork.kernel.org; Mon, 09 May 2016 08:40:56 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46038) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1azkUN-0000ny-2I for qemu-devel@nongnu.org; Mon, 09 May 2016 08:40:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1azkUF-0002mM-P3 for qemu-devel@nongnu.org; Mon, 09 May 2016 08:40:41 -0400 Received: from mx1.redhat.com ([209.132.183.28]:47699) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1azkUF-0002mC-FP for qemu-devel@nongnu.org; Mon, 09 May 2016 08:40:35 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 009646266A; Mon, 9 May 2016 12:40:35 +0000 (UTC) Received: from localhost (ovpn-112-46.ams2.redhat.com [10.36.112.46]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u49C8Md5022024; Mon, 9 May 2016 08:08:23 -0400 From: Stefan Hajnoczi To: qemu-devel@nongnu.org Date: Mon, 9 May 2016 13:07:50 +0100 Message-Id: <1462795687-25698-7-git-send-email-stefanha@redhat.com> In-Reply-To: <1462795687-25698-1-git-send-email-stefanha@redhat.com> References: <1462795687-25698-1-git-send-email-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Mon, 09 May 2016 12:40:35 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 06/23] qom: -object error messages lost location, restore it X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: marc.mari.barcelo@gmail.com, Paolo Bonzini , jsnow@redhat.com, Markus Armbruster Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Markus Armbruster qemu_opts_foreach() runs its callback with the error location set to the option's location. Any errors the callback reports use the option's location automatically. Commit 90998d5 moved the actual error reporting from "inside" qemu_opts_foreach() to after it. Here's a typical hunk: if (qemu_opts_foreach(qemu_find_opts("object"), - object_create, - object_create_initial, NULL)) { + user_creatable_add_opts_foreach, + object_create_initial, &err)) { + error_report_err(err); exit(1); } Before, object_create() reports from within qemu_opts_foreach(), using the option's location. Afterwards, we do it after qemu_opts_foreach(), using whatever location happens to be current there. Commonly a "none" location. This is because Error objects don't have location information. Problematic. Reproducer: $ qemu-system-x86_64 -nodefaults -display none -object secret,id=foo,foo=bar qemu-system-x86_64: Property '.foo' not found Note no location. This commit restores it: qemu-system-x86_64: -object secret,id=foo,foo=bar: Property '.foo' not found Note that the qemu_opts_foreach() bug just fixed could mask the bug here: if the location it leaves dangling hasn't been clobbered, yet, it's the correct one. Reported-by: Eric Blake Cc: Daniel P. Berrange Signed-off-by: Markus Armbruster Message-Id: <1461767349-15329-4-git-send-email-armbru@redhat.com> Reviewed-by: Daniel P. Berrange Reviewed-by: Eric Blake [Paragraph on Error added to commit message] --- include/qom/object_interfaces.h | 5 +++-- qemu-img.c | 39 +++++++++++---------------------------- qemu-io.c | 3 +-- qemu-nbd.c | 3 +-- qom/object_interfaces.c | 4 +++- vl.c | 6 ++---- 6 files changed, 21 insertions(+), 39 deletions(-) diff --git a/include/qom/object_interfaces.h b/include/qom/object_interfaces.h index d579746..8b17f4d 100644 --- a/include/qom/object_interfaces.h +++ b/include/qom/object_interfaces.h @@ -140,7 +140,7 @@ typedef bool (*user_creatable_add_opts_predicate)(const char *type); * user_creatable_add_opts_foreach: * @opaque: a user_creatable_add_opts_predicate callback or NULL * @opts: options to create - * @errp: if an error occurs, a pointer to an area to store the error + * @errp: unused * * An iterator callback to be used in conjunction with * the qemu_opts_foreach() method for creating a list of @@ -148,8 +148,9 @@ typedef bool (*user_creatable_add_opts_predicate)(const char *type); * * The @opaque parameter can be passed a user_creatable_add_opts_predicate * callback to filter which types of object are created during iteration. + * When it fails, report the error. * - * Returns: 0 on success, -1 on error + * Returns: 0 on success, -1 when an error was reported. */ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp); diff --git a/qemu-img.c b/qemu-img.c index 1697762..46f2a6d 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -435,8 +435,7 @@ static int img_create(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { goto fail; } @@ -598,7 +597,6 @@ static int img_check(int argc, char **argv) bool writethrough; ImageCheck *check; bool quiet = false; - Error *local_err = NULL; bool image_opts = false; fmt = NULL; @@ -679,8 +677,7 @@ static int img_check(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { return 1; } @@ -871,8 +868,7 @@ static int img_commit(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { return 1; } @@ -1133,7 +1129,6 @@ static int img_compare(int argc, char **argv) int64_t nb_sectors; int c, pnum; uint64_t progress_base; - Error *local_err = NULL; bool image_opts = false; cache = BDRV_DEFAULT_CACHE; @@ -1201,8 +1196,7 @@ static int img_compare(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { ret = 2; goto out4; } @@ -1864,8 +1858,7 @@ static int img_convert(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { goto fail_getopt; } @@ -2299,7 +2292,6 @@ static int img_info(int argc, char **argv) bool chain = false; const char *filename, *fmt, *output; ImageInfoList *list; - Error *local_err = NULL; bool image_opts = false; fmt = NULL; @@ -2363,8 +2355,7 @@ static int img_info(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { return 1; } @@ -2513,7 +2504,6 @@ static int img_map(int argc, char **argv) int64_t length; MapEntry curr = { .length = 0 }, next; int ret = 0; - Error *local_err = NULL; bool image_opts = false; fmt = NULL; @@ -2573,8 +2563,7 @@ static int img_map(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { return 1; } @@ -2717,8 +2706,7 @@ static int img_snapshot(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &err)) { - error_report_err(err); + NULL, NULL)) { return 1; } @@ -2867,8 +2855,7 @@ static int img_rebase(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { return 1; } @@ -3133,7 +3120,6 @@ static int img_resize(int argc, char **argv) bool quiet = false; BlockBackend *blk = NULL; QemuOpts *param; - Error *local_err = NULL; static QemuOptsList resize_options = { .name = "resize_options", @@ -3204,8 +3190,7 @@ static int img_resize(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { return 1; } @@ -3297,7 +3282,6 @@ static int img_amend(int argc, char **argv) bool quiet = false, progress = false; BlockBackend *blk = NULL; BlockDriverState *bs = NULL; - Error *local_err = NULL; bool image_opts = false; cache = BDRV_DEFAULT_CACHE; @@ -3365,8 +3349,7 @@ static int img_amend(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { ret = -1; goto out_no_progress; } diff --git a/qemu-io.c b/qemu-io.c index 288bba8..0598251 100644 --- a/qemu-io.c +++ b/qemu-io.c @@ -534,8 +534,7 @@ int main(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_error)) { - error_report_err(local_error); + NULL, NULL)) { exit(1); } diff --git a/qemu-nbd.c b/qemu-nbd.c index 2c9754e..c55b40f 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -711,8 +711,7 @@ int main(int argc, char **argv) if (qemu_opts_foreach(&qemu_object_opts, user_creatable_add_opts_foreach, - NULL, &local_err)) { - error_report_err(local_err); + NULL, NULL)) { exit(EXIT_FAILURE); } diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c index ab5da35..3931890 100644 --- a/qom/object_interfaces.c +++ b/qom/object_interfaces.c @@ -170,6 +170,7 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp) { bool (*type_predicate)(const char *) = opaque; Object *obj = NULL; + Error *err = NULL; const char *type; type = qemu_opt_get(opts, "qom-type"); @@ -178,8 +179,9 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts *opts, Error **errp) return 0; } - obj = user_creatable_add_opts(opts, errp); + obj = user_creatable_add_opts(opts, &err); if (!obj) { + error_report_err(err); return -1; } object_unref(obj); diff --git a/vl.c b/vl.c index 9df534f..5fd22cb 100644 --- a/vl.c +++ b/vl.c @@ -4291,8 +4291,7 @@ int main(int argc, char **argv, char **envp) if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_add_opts_foreach, - object_create_initial, &err)) { - error_report_err(err); + object_create_initial, NULL)) { exit(1); } @@ -4410,8 +4409,7 @@ int main(int argc, char **argv, char **envp) if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_add_opts_foreach, - object_create_delayed, &err)) { - error_report_err(err); + object_create_delayed, NULL)) { exit(1); }