On Tue, 12/19 16:45, Peter Xu wrote:
> After this patch, we will allow QMP clients to enable QMP capabilities
> when sending the first "qmp_capabilities" command. Originally we are
> starting QMP session with no arguments like:
>
> { "execute": "qmp_capabilities" }
>
> Now we can enable some QMP capabilities using (take OOB as example,
> which is the only one capability that we support):
>
> { "execute": "qmp_capabilities",
> "argument": { "enable": [ "oob" ] } }
>
> When the "argument" key is not provided, no capability is enabled.
>
> For capability "oob", the monitor needs to be run on dedicated IO
> thread, otherwise the command will fail. For example, trying to enable
> OOB on a MUXed typed QMP monitor will fail.
>
> One thing to mention is that, QMP capabilities are per-monitor, and also
> when the connection is closed due to some reason, the capabilities will
> be reset.
>
> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> monitor.c | 65
> +++++++++++++++++++++++++++++++++++++++++++++++++++++---
> qapi-schema.json | 15 ++++++++++---
> 2 files changed, 74 insertions(+), 6 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index 81fb0a42b4..c88a11cd69 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -166,6 +166,7 @@ typedef struct {
> * mode.
> */
> QmpCommandList *commands;
> + bool qmp_caps[QMP_CAPABILITY__MAX];
> } MonitorQMP;
>
> /*
> @@ -1035,8 +1036,42 @@ static void monitor_init_qmp_commands(void)
> qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS);
> }
>
> -void qmp_qmp_capabilities(Error **errp)
> +static void qmp_caps_check(Monitor *mon, QMPCapabilityList *list,
> + Error **errp)
> +{
> + for (; list; list = list->next) {
> + assert(list->value < QMP_CAPABILITY__MAX);
> + switch (list->value) {
> + case QMP_CAPABILITY_OOB:
> + if (!mon->use_io_thr) {
> + /*
> + * Out-Of-Band only works with monitors that are
> + * running on dedicated IOThread.
> + */
> + error_setg(errp, "This monitor does not support "
> + "Out-Of-Band (OOB)");
> + return;
> + }
> + break;
> + default:
> + break;
> + }
> + }
> +}
> +
> +/* This function should only be called after capabilities are checked. */
> +static void qmp_caps_apply(Monitor *mon, QMPCapabilityList *list)
> +{
> + for (; list; list = list->next) {
> + mon->qmp.qmp_caps[list->value] = true;
> + }
> +}
> +
> +void qmp_qmp_capabilities(bool has_enable, QMPCapabilityList *enable,
> + Error **errp)
> {
> + Error *local_err = NULL;
> +
> if (cur_mon->qmp.commands == &qmp_commands) {
> error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> "Capabilities negotiation is already complete, command "
> @@ -1044,6 +1079,20 @@ void qmp_qmp_capabilities(Error **errp)
> return;
> }
>
> + /* Enable QMP capabilities provided by the guest if applicable. */
s/guest/client/ ?
> + if (has_enable) {
> + qmp_caps_check(cur_mon, enable, &local_err);
> + if (local_err) {
> + /*
> + * Failed check on either of the capabilities will fail
> + * the apply of all.
> + */
> + error_propagate(errp, local_err);
> + return;
> + }
> + qmp_caps_apply(cur_mon, enable);
> + }
> +
> cur_mon->qmp.commands = &qmp_commands;
> }
>
> @@ -3941,7 +3990,7 @@ void monitor_resume(Monitor *mon)
> readline_show_prompt(mon->rs);
> }
>
> -static QObject *get_qmp_greeting(void)
> +static QObject *get_qmp_greeting(Monitor *mon)
> {
> QList *cap_list = qlist_new();
> QObject *ver = NULL;
> @@ -3950,6 +3999,10 @@ static QObject *get_qmp_greeting(void)
> qmp_marshal_query_version(NULL, &ver, NULL);
>
> for (cap = 0; cap < QMP_CAPABILITY__MAX; cap++) {
> + if (!mon->use_io_thr && cap == QMP_CAPABILITY_OOB) {
> + /* Monitors that are not using IOThread won't support OOB */
> + continue;
> + }
OK, I thought this could better go to the previous patch, but it may be fine
here, together with monitor_qmp_caps_reset().
> qlist_append(cap_list, qstring_from_str(QMPCapability_str(cap)));
> }
>
> @@ -3957,6 +4010,11 @@ static QObject *get_qmp_greeting(void)
> ver, cap_list);
> }
>
> +static void monitor_qmp_caps_reset(Monitor *mon)
> +{
> + memset(mon->qmp.qmp_caps, 0, sizeof(mon->qmp.qmp_caps));
> +}
> +
> static void monitor_qmp_event(void *opaque, int event)
> {
> QObject *data;
> @@ -3965,7 +4023,8 @@ static void monitor_qmp_event(void *opaque, int event)
> switch (event) {
> case CHR_EVENT_OPENED:
> mon->qmp.commands = &qmp_cap_negotiation_commands;
> - data = get_qmp_greeting();
> + monitor_qmp_caps_reset(mon);
> + data = get_qmp_greeting(mon);
> monitor_json_emitter(mon, data);
> qobject_decref(data);
> mon_refcount++;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 03201578b4..531fd4c0db 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -102,21 +102,30 @@
> #
> # Enable QMP capabilities.
> #
> -# Arguments: None.
> +# Arguments:
> +#
> +# @enable: List of QMPCapabilities to enable, which is optional.
> +# If not provided, it means no QMP capabilities will be
> +# enabled. (since 2.12)
> #
> # Example:
> #
> -# -> { "execute": "qmp_capabilities" }
> +# -> { "execute": "qmp_capabilities",
> +# "arguments": { "enable": [ "oob" ] } }
> # <- { "return": {} }
> #
> # Notes: This command is valid exactly when first connecting: it must be
> # issued before any other command will be accepted, and will fail once the
> # monitor is accepting other commands. (see qemu docs/interop/qmp-spec.txt)
> #
> +# QMP client needs to explicitly enable QMP capabilities, otherwise
> +# all the QMP capabilities will be turned off by default.
> +#
> # Since: 0.13
> #
> ##
> -{ 'command': 'qmp_capabilities' }
> +{ 'command': 'qmp_capabilities',
> + 'data': { '*enable': [ 'QMPCapability' ] } }
>
> ##
> # @QMPCapability:
> --
> 2.14.3
>
Fam
