From patchwork Tue Sep 20 22:00:39 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Marc-Andr=C3=A9_Lureau?= X-Patchwork-Id: 9342545 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id A799C607EE for ; Tue, 20 Sep 2016 22:01:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 967CB297D1 for ; Tue, 20 Sep 2016 22:01:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8AE2D297E8; Tue, 20 Sep 2016 22:01:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 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.wl.linuxfoundation.org (Postfix) with ESMTPS id ECE1D297D1 for ; Tue, 20 Sep 2016 22:01:56 +0000 (UTC) Received: from localhost ([::1]:38443 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmT6x-0004uS-Qy for patchwork-qemu-devel@patchwork.kernel.org; Tue, 20 Sep 2016 18:01:55 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34428) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmT6U-0004s0-Ox for qemu-devel@nongnu.org; Tue, 20 Sep 2016 18:01:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bmT6Q-0002Jr-Iz for qemu-devel@nongnu.org; Tue, 20 Sep 2016 18:01:25 -0400 Received: from mx5-phx2.redhat.com ([209.132.183.37]:32942) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmT6Q-0002Je-6Y for qemu-devel@nongnu.org; Tue, 20 Sep 2016 18:01:22 -0400 Received: from zmail17.collab.prod.int.phx2.redhat.com (zmail17.collab.prod.int.phx2.redhat.com [10.5.83.19]) by mx5-phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u8KM0dRB006828; Tue, 20 Sep 2016 18:00:40 -0400 Date: Tue, 20 Sep 2016 18:00:39 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau To: Alberto Garcia Message-ID: <81179735.279206.1474408839927.JavaMail.zimbra@redhat.com> In-Reply-To: <1441973931.277653.1474408728887.JavaMail.zimbra@redhat.com> References: <20160912091913.15831-1-marcandre.lureau@redhat.com> <20160912091913.15831-16-marcandre.lureau@redhat.com> <20160920144844.nzbpfrauczlml3a5@perseus.local> <1441973931.277653.1474408728887.JavaMail.zimbra@redhat.com> MIME-Version: 1.0 X-Originating-IP: [10.3.116.87] X-Mailer: Zimbra 8.0.6_GA_5922 (ZimbraWebClient - FF48 (Linux)/8.0.6_GA_5922) Thread-Topic: monitor: use qmp_dispatch() Thread-Index: MvWAhg5J14dIYu8G6MPCzJXlx6UHsRXLd55+ X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.37 Subject: Re: [Qemu-devel] [PATCH v6 15/18] monitor: use qmp_dispatch() 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: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau , qemu-devel@nongnu.org, armbru@redhat.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP ----- Original Message ----- > Hi Alberto > > ----- Original Message ----- > > On Mon, Sep 12, 2016 at 01:19:10PM +0400, Marc-André Lureau wrote: > > > Replace the old manual dispatch and validation code by the generic one > > > provided by qapi common code. > > > > > > Note that it is now possible to call the following commands that used to > > > be disabled by compile-time conditionals: > > > - dump-skeys > > > - query-spice > > > - rtc-reset-reinjection > > > - query-gic-capabilities > > > > > > Their fallback functions return an appropriate "feature disabled" error. > > > > > > Signed-off-by: Marc-André Lureau > > > > This patch breaks iotest 085 because the "missing parameter" error is > > now different: > > > > -{"error": {"class": "GenericError", "desc": "Parameter 'snapshot-file' is > > missing"}} > > +{"error": {"class": "GenericError", "desc": "Invalid parameter type for > > 'snapshot-file', expected: string"}} > > > > I was thinking to update the expected output of the iotest, but I > > guess it's better to return a more meaningful error message? > > The change is relatively easy (see attached patch), but there are other tests > that expected the current error, see commit fe509ee237307843. I guess we > should decided with one, and I think missing parameter is more appropriate. > diff attached diff --git a/qapi/qmp-input-visitor.c b/qapi/qmp-input-visitor.c index 64dd392..22b132b 100644 --- a/qapi/qmp-input-visitor.c +++ b/qapi/qmp-input-visitor.c @@ -169,7 +169,11 @@ static void qmp_input_start_struct(Visitor *v, const char *name, void **obj, if (obj) { *obj = NULL; } - if (!qobj || qobject_type(qobj) != QTYPE_QDICT) { + if (!qobj) { + error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } + if (qobject_type(qobj) != QTYPE_QDICT) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "QDict"); return; @@ -194,7 +198,11 @@ static void qmp_input_start_list(Visitor *v, const char *name, QObject *qobj = qmp_input_get_object(qiv, name, true); const QListEntry *entry; - if (!qobj || qobject_type(qobj) != QTYPE_QLIST) { + if (!qobj) { + error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } + if (qobject_type(qobj) != QTYPE_QLIST) { if (list) { *list = NULL; } @@ -234,8 +242,10 @@ static void qmp_input_start_alternate(Visitor *v, const char *name, QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, false); - if (!qobj) { + if (obj) { *obj = NULL; + } + if (!qobj) { error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); return; } @@ -250,8 +260,13 @@ static void qmp_input_type_int64(Visitor *v, const char *name, int64_t *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true); + QInt *qint = qobject_to_qint(qobj); + if (!qobj) { + error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } if (!qint) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer"); @@ -266,8 +281,13 @@ static void qmp_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, { /* FIXME: qobject_to_qint mishandles values over INT64_MAX */ QmpInputVisitor *qiv = to_qiv(v); - QInt *qint = qobject_to_qint(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true); + QInt *qint = qobject_to_qint(qobj); + if (!qobj) { + error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } if (!qint) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "integer"); @@ -281,8 +301,13 @@ static void qmp_input_type_bool(Visitor *v, const char *name, bool *obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QBool *qbool = qobject_to_qbool(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true); + QBool *qbool = qobject_to_qbool(qobj); + if (!qobj) { + error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } if (!qbool) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "boolean"); @@ -296,8 +321,16 @@ static void qmp_input_type_str(Visitor *v, const char *name, char **obj, Error **errp) { QmpInputVisitor *qiv = to_qiv(v); - QString *qstr = qobject_to_qstring(qmp_input_get_object(qiv, name, true)); + QObject *qobj = qmp_input_get_object(qiv, name, true); + QString *qstr = qobject_to_qstring(qobj); + if (obj) { + *obj = NULL; + } + if (!qobj) { + error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } if (!qstr) { *obj = NULL; error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", @@ -316,6 +349,10 @@ static void qmp_input_type_number(Visitor *v, const char *name, double *obj, QInt *qint; QFloat *qfloat; + if (!qobj) { + error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } qint = qobject_to_qint(qobj); if (qint) { *obj = qint_get_int(qobject_to_qint(qobj)); @@ -338,6 +375,14 @@ static void qmp_input_type_any(Visitor *v, const char *name, QObject **obj, QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, true); + if (obj) { + *obj = NULL; + } + if (!qobj) { + error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } + qobject_incref(qobj); *obj = qobj; } @@ -347,6 +392,10 @@ static void qmp_input_type_null(Visitor *v, const char *name, Error **errp) QmpInputVisitor *qiv = to_qiv(v); QObject *qobj = qmp_input_get_object(qiv, name, true); + if (!qobj) { + error_setg(errp, QERR_MISSING_PARAMETER, name ? name : "null"); + return; + } if (qobject_type(qobj) != QTYPE_QNULL) { error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", "null");