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.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
monitor.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
qapi-schema.json | 15 ++++++++++++---
2 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/monitor.c b/monitor.c
index bad6ee8dd1..097312e65f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -167,6 +167,7 @@ typedef struct {
* mode.
*/
QmpCommandList *commands;
+ bool qmp_caps[QMP_CAPABILITY__MAX];
} MonitorQMP;
/*
@@ -1037,8 +1038,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 "
@@ -1046,6 +1081,20 @@ void qmp_qmp_capabilities(Error **errp)
return;
}
+ /* Enable QMP capabilities provided by the guest if applicable. */
+ 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;
}
@@ -3963,6 +4012,11 @@ static QObject *get_qmp_greeting(void)
return QOBJECT(result);
}
+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;
@@ -3971,6 +4025,7 @@ static void monitor_qmp_event(void *opaque, int event)
switch (event) {
case CHR_EVENT_OPENED:
mon->qmp.commands = &qmp_cap_negotiation_commands;
+ monitor_qmp_caps_reset(mon);
data = get_qmp_greeting();
monitor_json_emitter(mon, data);
qobject_decref(data);
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
* Peter Xu (peterx@redhat.com) 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.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
Typo in the title nego*t*iate, other than that.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> monitor.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> qapi-schema.json | 15 ++++++++++++---
> 2 files changed, 68 insertions(+), 4 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index bad6ee8dd1..097312e65f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -167,6 +167,7 @@ typedef struct {
> * mode.
> */
> QmpCommandList *commands;
> + bool qmp_caps[QMP_CAPABILITY__MAX];
> } MonitorQMP;
>
> /*
> @@ -1037,8 +1038,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 "
> @@ -1046,6 +1081,20 @@ void qmp_qmp_capabilities(Error **errp)
> return;
> }
>
> + /* Enable QMP capabilities provided by the guest if applicable. */
> + 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;
> }
>
> @@ -3963,6 +4012,11 @@ static QObject *get_qmp_greeting(void)
> return QOBJECT(result);
> }
>
> +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;
> @@ -3971,6 +4025,7 @@ static void monitor_qmp_event(void *opaque, int event)
> switch (event) {
> case CHR_EVENT_OPENED:
> mon->qmp.commands = &qmp_cap_negotiation_commands;
> + monitor_qmp_caps_reset(mon);
> data = get_qmp_greeting();
> monitor_json_emitter(mon, data);
> qobject_decref(data);
> 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
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Tue, Dec 05, 2017 at 01:51:46PM +0800, Peter Xu wrote:
> @@ -1037,8 +1038,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;
QEMU always offers the 'oob' capability, even if the monitor does not
support it. Should it send 'oob' only when mon->use_io_thr to make
things easier for clients?
On Wed, 12/13 17:19, Stefan Hajnoczi wrote:
> On Tue, Dec 05, 2017 at 01:51:46PM +0800, Peter Xu wrote:
> > @@ -1037,8 +1038,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;
>
> QEMU always offers the 'oob' capability, even if the monitor does not
> support it. Should it send 'oob' only when mon->use_io_thr to make
> things easier for clients?
So should we firstly agree on whether the capabilities is on the current monitor
connection or QEMU as a whole?
Fam
On Fri, Dec 15, 2017 at 05:40:10PM +0800, Fam Zheng wrote:
> On Wed, 12/13 17:19, Stefan Hajnoczi wrote:
> > On Tue, Dec 05, 2017 at 01:51:46PM +0800, Peter Xu wrote:
> > > @@ -1037,8 +1038,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;
> >
> > QEMU always offers the 'oob' capability, even if the monitor does not
> > support it. Should it send 'oob' only when mon->use_io_thr to make
> > things easier for clients?
>
> So should we firstly agree on whether the capabilities is on the current monitor
> connection or QEMU as a whole?
It's more flexible to allow per-connection capabilities. Is there a
reason against it?
Stefan
On Fri, 12/15 13:26, Stefan Hajnoczi wrote: > > > QEMU always offers the 'oob' capability, even if the monitor does not > > > support it. Should it send 'oob' only when mon->use_io_thr to make > > > things easier for clients? > > > > So should we firstly agree on whether the capabilities is on the current monitor > > connection or QEMU as a whole? > > It's more flexible to allow per-connection capabilities. Is there a > reason against it? No, I just think either way we should document it. So if we define it per-connection, like you said, "oob" shouldn't be sent in the greeting message, and patch 11 need to be updated. Fam
On Fri, Dec 15, 2017 at 09:53:53PM +0800, Fam Zheng wrote: > On Fri, 12/15 13:26, Stefan Hajnoczi wrote: > > > > QEMU always offers the 'oob' capability, even if the monitor does not > > > > support it. Should it send 'oob' only when mon->use_io_thr to make > > > > things easier for clients? Agree. > > > > > > So should we firstly agree on whether the capabilities is on the current monitor > > > connection or QEMU as a whole? > > > > It's more flexible to allow per-connection capabilities. Is there a > > reason against it? > > No, I just think either way we should document it. So if we define it > per-connection, like you said, "oob" shouldn't be sent in the greeting message, > and patch 11 need to be updated. Yeah, that's where I'll do the update. Thanks! -- Peter Xu
© 2016 - 2026 Red Hat, Inc.