From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43894) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZfK7-0004gG-90 for qemu-devel@nongnu.org; Tue, 16 Aug 2016 10:26:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bZfK2-0006Hi-6S for qemu-devel@nongnu.org; Tue, 16 Aug 2016 10:26:34 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51606) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZfK1-0006HW-UU for qemu-devel@nongnu.org; Tue, 16 Aug 2016 10:26:30 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (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 7450D8535A for ; Tue, 16 Aug 2016 14:26:28 +0000 (UTC) From: Markus Armbruster References: <20160810180235.32469-1-marcandre.lureau@redhat.com> <20160810180235.32469-7-marcandre.lureau@redhat.com> Date: Tue, 16 Aug 2016 16:26:26 +0200 In-Reply-To: <20160810180235.32469-7-marcandre.lureau@redhat.com> (marcandre lureau's message of "Wed, 10 Aug 2016 22:02:24 +0400") Message-ID: <8737m44wod.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v4 06/17] monitor: unregister conditional commands List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: marcandre.lureau@redhat.com Cc: qemu-devel@nongnu.org marcandre.lureau@redhat.com writes: > From: Marc-Andr=C3=A9 Lureau > > The current monitor dispatch codes doesn't know commands that have been > filtered out during qmp-commands.hx preprocessing. query-commands > doesn't list them either. However, qapi generator doesn't filter them > out, and they are listed in the command list. > > For now, disable the commands that aren't avaible to avoid introducing a > regression there when switching to qmp_dispatch() or listing commands > from the generated qapi code. > > Signed-off-by: Marc-Andr=C3=A9 Lureau > --- > monitor.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/monitor.c b/monitor.c > index b7ae552..ef946ad 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -1008,6 +1008,26 @@ static void qmp_query_qmp_schema(QDict *qdict, QOb= ject **ret_data, > *ret_data =3D qobject_from_json(qmp_schema_json); > } >=20=20 > +/* > + * Those commands are registered unconditionnally by generated > + * qmp files. FIXME: Educate the QAPI schema on #ifdef commands. > + */ > +static void qmp_disable_marshal(void) > +{ > +#ifndef CONFIG_SPICE > + qmp_disable_command("query-spice"); > +#endif > +#ifndef TARGET_I386 > + qmp_disable_command("rtc-reset-reinjection"); > +#endif > +#ifndef TARGET_S390X > + qmp_disable_command("dump-skeys"); > +#endif > +#ifndef TARGET_ARM > + qmp_disable_command("query-gic-capabilities"); > +#endif > +} > + > static void qmp_init_marshal(void) > { > qmp_register_command("query-qmp-schema", qmp_query_qmp_schema, > @@ -1016,6 +1036,9 @@ static void qmp_init_marshal(void) > QCO_NO_OPTIONS); > qmp_register_command("netdev_add", qmp_netdev_add, > QCO_NO_OPTIONS); > + > + /* call it after the rest of qapi_init() */ > + register_module_init(qmp_disable_marshal, MODULE_INIT_QAPI); > } >=20=20 > qapi_init(qmp_init_marshal); Let's see whether I understand this patch... qmp_disable_command() sets a flag that can be tested directly or with qmp_command_is_enabled(). do_qmp_dispatch() tests it, and fails the command with an "The command FOO has been disabled for this instance" error. It's called from qmp_dispatch(), which we're not yet using for QMP at this point of the series. QGA uses this to implement its "freeze whitelist", i.e. to disable all commands that aren't known to be safe while filesystems are frozen. qmp_disable_command() does nothing when the command doesn't exist, which I think is the case at this point of the series. So this patch does exactly nothing by itself. When you later flip dispatch to use qmp_dispatch(), it becomes active, and the error for these commands changes from {"error": {"class": "CommandNotFound", "desc": "The command FOO has not= been found"}} to {"error": {"class": "GenericError", "desc": "The command FOO has been d= isabled for this instance"}} which is fine with me. Is this basically correct?