* [Qemu-devel] [PULL 01/14] qmp-test: fix response leak
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 02/14] tests: Silence false positive warning on generated test name Eric Blake
` (13 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, Markus Armbruster
From: Marc-André Lureau <marcandre.lureau@redhat.com>
Apparently introduced in commit a4f90923b520f1dc0a768634877eb412e5052c26.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180326172041.21009-1-marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/qmp-test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 558e83540cc..8de99a4727e 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -90,6 +90,7 @@ static void test_qmp_protocol(void)
test_version(qdict_get(q, "version"));
capabilities = qdict_get_qlist(q, "capabilities");
g_assert(capabilities && qlist_empty(capabilities));
+ QDECREF(resp);
/* Test valid command before handshake */
resp = qtest_qmp(qts, "{ 'execute': 'query-version' }");
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 02/14] tests: Silence false positive warning on generated test name
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 01/14] qmp-test: fix response leak Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 03/14] error: Strip trailing '\n' from error string arguments (again again) Eric Blake
` (12 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster, Michael Roth
Running 'make check' on rawhide with gcc 8.0.1 fails:
tests/test-visitor-serialization.c: In function 'main':
tests/test-visitor-serialization.c:1127:34: error: '/primitives/' directive writing 12 bytes into a region of size between 1 and 128 [-Werror=format-overflow=]
The warning is a false positive (we have two buffers of size 128,
so yes, if we FULLY used the first buffer, then sprint'ing it into
the second will overflow the second). But in practice, our first
buffer will not be longer than "/visitor/serialization/String",
so sizing it smaller is enough to let gcc see that we don't
overflow the second.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180323204341.1501664-1-eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/test-visitor-serialization.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/test-visitor-serialization.c b/tests/test-visitor-serialization.c
index 438c18a0d64..d18d90db2c7 100644
--- a/tests/test-visitor-serialization.c
+++ b/tests/test-visitor-serialization.c
@@ -1115,7 +1115,7 @@ static const SerializeOps visitors[] = {
static void add_visitor_type(const SerializeOps *ops)
{
- char testname_prefix[128];
+ char testname_prefix[32];
char testname[128];
TestArgs *args;
int i = 0;
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 03/14] error: Strip trailing '\n' from error string arguments (again again)
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 01/14] qmp-test: fix response leak Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 02/14] tests: Silence false positive warning on generated test name Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 04/14] error: Remove NULL checks on error_propagate() calls Eric Blake
` (11 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Paolo Bonzini, Richard Henderson, Eduardo Habkost
From: Laurent Vivier <lvivier@redhat.com>
Re-run Coccinelle script scripts/coccinelle/err-bad-newline.cocci,
and found new error_report() occurrences with '\n'.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20180323143202.28879-3-lvivier@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
target/i386/hvf/hvf.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 15870a4f366..c36753954be 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -86,25 +86,25 @@ static void assert_hvf_ok(hv_return_t ret)
switch (ret) {
case HV_ERROR:
- error_report("Error: HV_ERROR\n");
+ error_report("Error: HV_ERROR");
break;
case HV_BUSY:
- error_report("Error: HV_BUSY\n");
+ error_report("Error: HV_BUSY");
break;
case HV_BAD_ARGUMENT:
- error_report("Error: HV_BAD_ARGUMENT\n");
+ error_report("Error: HV_BAD_ARGUMENT");
break;
case HV_NO_RESOURCES:
- error_report("Error: HV_NO_RESOURCES\n");
+ error_report("Error: HV_NO_RESOURCES");
break;
case HV_NO_DEVICE:
- error_report("Error: HV_NO_DEVICE\n");
+ error_report("Error: HV_NO_DEVICE");
break;
case HV_UNSUPPORTED:
- error_report("Error: HV_UNSUPPORTED\n");
+ error_report("Error: HV_UNSUPPORTED");
break;
default:
- error_report("Unknown Error\n");
+ error_report("Unknown Error");
}
abort();
@@ -191,7 +191,7 @@ void hvf_set_phys_mem(MemoryRegionSection *section, bool add)
if (mem) {
mem->size = 0;
if (do_hvf_set_memory(mem)) {
- error_report("Failed to reset overlapping slot\n");
+ error_report("Failed to reset overlapping slot");
abort();
}
}
@@ -211,7 +211,7 @@ void hvf_set_phys_mem(MemoryRegionSection *section, bool add)
}
if (x == hvf_state->num_slots) {
- error_report("No free slots\n");
+ error_report("No free slots");
abort();
}
@@ -221,7 +221,7 @@ void hvf_set_phys_mem(MemoryRegionSection *section, bool add)
mem->region = area;
if (do_hvf_set_memory(mem)) {
- error_report("Error registering new memory slot\n");
+ error_report("Error registering new memory slot");
abort();
}
}
@@ -884,7 +884,7 @@ int hvf_vcpu_exec(CPUState *cpu)
break;
}
default:
- error_report("Unrecognized CR %d\n", cr);
+ error_report("Unrecognized CR %d", cr);
abort();
}
RIP(env) += ins_len;
@@ -930,7 +930,7 @@ int hvf_vcpu_exec(CPUState *cpu)
env->error_code = 0;
break;
default:
- error_report("%llx: unhandled exit %llx\n", rip, exit_reason);
+ error_report("%llx: unhandled exit %llx", rip, exit_reason);
}
} while (ret == 0);
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 04/14] error: Remove NULL checks on error_propagate() calls
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (2 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 03/14] error: Strip trailing '\n' from error string arguments (again again) Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 05/14] qdict: remove useless cast Eric Blake
` (10 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Laurent Vivier, Daniel P. Berrangé
From: Laurent Vivier <lvivier@redhat.com>
Re-run Coccinelle patch
scripts/coccinelle/error_propagate_null.cocci
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20180323143202.28879-4-lvivier@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
io/channel-websock.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/io/channel-websock.c b/io/channel-websock.c
index ec48a305f02..e6608b969d7 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -586,9 +586,7 @@ static gboolean qio_channel_websock_handshake_io(QIOChannel *ioc,
return TRUE;
}
- if (err) {
- error_propagate(&wioc->io_err, err);
- }
+ error_propagate(&wioc->io_err, err);
trace_qio_channel_websock_handshake_reply(ioc);
qio_channel_add_watch(
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 05/14] qdict: remove useless cast
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (3 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 04/14] error: Remove NULL checks on error_propagate() calls Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 06/14] qmp: fix qmp_capabilities error regression Eric Blake
` (9 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Fam Zheng, Kevin Wolf, Max Reitz,
Markus Armbruster, Dr. David Alan Gilbert,
open list:NVMe Block Driver
From: Laurent Vivier <lvivier@redhat.com>
Re-run Coccinelle script scripts/coccinelle/qobject.cocci
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Message-Id: <20180323143202.28879-5-lvivier@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Acked-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
block/nvme.c | 11 +++++------
monitor.c | 2 +-
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/block/nvme.c b/block/nvme.c
index 8bca57aae69..c4f3a7bc94a 100644
--- a/block/nvme.c
+++ b/block/nvme.c
@@ -695,12 +695,11 @@ static void nvme_parse_filename(const char *filename, QDict *options,
unsigned long ns;
const char *slash = strchr(tmp, '/');
if (!slash) {
- qdict_put(options, NVME_BLOCK_OPT_DEVICE,
- qstring_from_str(tmp));
+ qdict_put_str(options, NVME_BLOCK_OPT_DEVICE, tmp);
return;
}
device = g_strndup(tmp, slash - tmp);
- qdict_put(options, NVME_BLOCK_OPT_DEVICE, qstring_from_str(device));
+ qdict_put_str(options, NVME_BLOCK_OPT_DEVICE, device);
g_free(device);
namespace = slash + 1;
if (*namespace && qemu_strtoul(namespace, NULL, 10, &ns)) {
@@ -708,8 +707,8 @@ static void nvme_parse_filename(const char *filename, QDict *options,
namespace);
return;
}
- qdict_put(options, NVME_BLOCK_OPT_NAMESPACE,
- qstring_from_str(*namespace ? namespace : "1"));
+ qdict_put_str(options, NVME_BLOCK_OPT_NAMESPACE,
+ *namespace ? namespace : "1");
}
}
@@ -1082,7 +1081,7 @@ static void nvme_refresh_filename(BlockDriverState *bs, QDict *opts)
bs->drv->format_name);
}
- qdict_put(opts, "driver", qstring_from_str(bs->drv->format_name));
+ qdict_put_str(opts, "driver", bs->drv->format_name);
bs->full_open_options = opts;
}
diff --git a/monitor.c b/monitor.c
index 77f4c41cfa6..de709fc2e5d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4315,7 +4315,7 @@ static QObject *get_qmp_greeting(Monitor *mon)
/* Monitors that are not using IOThread won't support OOB */
continue;
}
- qlist_append(cap_list, qstring_from_str(QMPCapability_str(cap)));
+ qlist_append_str(cap_list, QMPCapability_str(cap));
}
return qobject_from_jsonf("{'QMP': {'version': %p, 'capabilities': %p}}",
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 06/14] qmp: fix qmp_capabilities error regression
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (4 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 05/14] qdict: remove useless cast Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 15:01 ` Marc-André Lureau
2018-03-27 14:30 ` [Qemu-devel] [PULL 07/14] qapi: restrict allow-oob value to be "true" Eric Blake
` (8 subsequent siblings)
14 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Dr. David Alan Gilbert, Markus Armbruster
From: Peter Xu <peterx@redhat.com>
When someone sends a command before QMP handshake, the error used to be
like this:
{"execute": "query-cpus"}
{"error": {"class": "CommandNotFound", "desc":
"Expecting capabilities negotiation with 'qmp_capabilities'"}}
While after cf869d5317 it becomes:
{"execute": "query-cpus"}
{"error": {"class": "CommandNotFound", "desc":
"The command query-cpus has not been found"}}
Fix it back to the nicer one.
Fixes: cf869d5317 ("qmp: support out-of-band (oob) execution", 2018-03-19)
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-2-peterx@redhat.com>
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: commit message grammar tweaks]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
monitor.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/monitor.c b/monitor.c
index de709fc2e5d..3b1ef34711b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1203,8 +1203,14 @@ static bool qmp_cmd_oob_check(Monitor *mon, QDict *req, Error **errp)
cmd = qmp_find_command(mon->qmp.commands, command);
if (!cmd) {
- error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
- "The command %s has not been found", command);
+ if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
+ error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+ "Expecting capabilities negotiation "
+ "with 'qmp_capabilities'");
+ } else {
+ error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
+ "The command %s has not been found", command);
+ }
return false;
}
@@ -4027,7 +4033,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
{
Monitor *mon, *old_mon;
QObject *req, *rsp = NULL, *id;
- QDict *qdict = NULL;
bool need_resume;
req = req_obj->req;
@@ -4050,18 +4055,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
cur_mon = old_mon;
- if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
- qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error");
- if (qdict
- && !g_strcmp0(qdict_get_try_str(qdict, "class"),
- QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) {
- /* Provide a more useful error message */
- qdict_del(qdict, "desc");
- qdict_put_str(qdict, "desc", "Expecting capabilities negotiation"
- " with 'qmp_capabilities'");
- }
- }
-
/* Respond if necessary */
monitor_qmp_respond(mon, rsp, NULL, id);
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PULL 06/14] qmp: fix qmp_capabilities error regression
2018-03-27 14:30 ` [Qemu-devel] [PULL 06/14] qmp: fix qmp_capabilities error regression Eric Blake
@ 2018-03-27 15:01 ` Marc-André Lureau
2018-03-27 15:16 ` Eric Blake
0 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2018-03-27 15:01 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU, Dr. David Alan Gilbert, Peter Xu, Markus Armbruster
On Tue, Mar 27, 2018 at 4:30 PM, Eric Blake <eblake@redhat.com> wrote:
> From: Peter Xu <peterx@redhat.com>
>
> When someone sends a command before QMP handshake, the error used to be
> like this:
>
> {"execute": "query-cpus"}
> {"error": {"class": "CommandNotFound", "desc":
> "Expecting capabilities negotiation with 'qmp_capabilities'"}}
>
> While after cf869d5317 it becomes:
>
> {"execute": "query-cpus"}
> {"error": {"class": "CommandNotFound", "desc":
> "The command query-cpus has not been found"}}
>
> Fix it back to the nicer one.
>
> Fixes: cf869d5317 ("qmp: support out-of-band (oob) execution", 2018-03-19)
> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> Message-Id: <20180326063901.27425-2-peterx@redhat.com>
> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by (not x2 Reported-by ;)
> Reviewed-by: Eric Blake <eblake@redhat.com>
> [eblake: commit message grammar tweaks]
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
> monitor.c | 23 ++++++++---------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/monitor.c b/monitor.c
> index de709fc2e5d..3b1ef34711b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1203,8 +1203,14 @@ static bool qmp_cmd_oob_check(Monitor *mon, QDict *req, Error **errp)
>
> cmd = qmp_find_command(mon->qmp.commands, command);
> if (!cmd) {
> - error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> - "The command %s has not been found", command);
> + if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
> + error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> + "Expecting capabilities negotiation "
> + "with 'qmp_capabilities'");
> + } else {
> + error_set(errp, ERROR_CLASS_COMMAND_NOT_FOUND,
> + "The command %s has not been found", command);
> + }
> return false;
> }
>
> @@ -4027,7 +4033,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
> {
> Monitor *mon, *old_mon;
> QObject *req, *rsp = NULL, *id;
> - QDict *qdict = NULL;
> bool need_resume;
>
> req = req_obj->req;
> @@ -4050,18 +4055,6 @@ static void monitor_qmp_dispatch_one(QMPRequest *req_obj)
>
> cur_mon = old_mon;
>
> - if (mon->qmp.commands == &qmp_cap_negotiation_commands) {
> - qdict = qdict_get_qdict(qobject_to(QDict, rsp), "error");
> - if (qdict
> - && !g_strcmp0(qdict_get_try_str(qdict, "class"),
> - QapiErrorClass_str(ERROR_CLASS_COMMAND_NOT_FOUND))) {
> - /* Provide a more useful error message */
> - qdict_del(qdict, "desc");
> - qdict_put_str(qdict, "desc", "Expecting capabilities negotiation"
> - " with 'qmp_capabilities'");
> - }
> - }
> -
> /* Respond if necessary */
> monitor_qmp_respond(mon, rsp, NULL, id);
>
> --
> 2.14.3
>
>
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PULL 06/14] qmp: fix qmp_capabilities error regression
2018-03-27 15:01 ` Marc-André Lureau
@ 2018-03-27 15:16 ` Eric Blake
2018-03-27 15:31 ` Marc-André Lureau
0 siblings, 1 reply; 19+ messages in thread
From: Eric Blake @ 2018-03-27 15:16 UTC (permalink / raw)
To: Marc-André Lureau
Cc: QEMU, Dr. David Alan Gilbert, Peter Xu, Markus Armbruster
On 03/27/2018 10:01 AM, Marc-André Lureau wrote:
> On Tue, Mar 27, 2018 at 4:30 PM, Eric Blake <eblake@redhat.com> wrote:
>> From: Peter Xu <peterx@redhat.com>
>>
>> When someone sends a command before QMP handshake, the error used to be
>> like this:
>>
>> {"execute": "query-cpus"}
>> {"error": {"class": "CommandNotFound", "desc":
>> "Expecting capabilities negotiation with 'qmp_capabilities'"}}
>>
>> While after cf869d5317 it becomes:
>>
>> {"execute": "query-cpus"}
>> {"error": {"class": "CommandNotFound", "desc":
>> "The command query-cpus has not been found"}}
>>
>> Fix it back to the nicer one.
>>
>> Fixes: cf869d5317 ("qmp: support out-of-band (oob) execution", 2018-03-19)
>> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Signed-off-by: Peter Xu <peterx@redhat.com>
>> Message-Id: <20180326063901.27425-2-peterx@redhat.com>
>> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Reviewed-by (not x2 Reported-by ;)
>
Shoot. Want me to send a v2 pull request?
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PULL 06/14] qmp: fix qmp_capabilities error regression
2018-03-27 15:16 ` Eric Blake
@ 2018-03-27 15:31 ` Marc-André Lureau
0 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2018-03-27 15:31 UTC (permalink / raw)
To: Eric Blake; +Cc: QEMU, Dr. David Alan Gilbert, Peter Xu, Markus Armbruster
On Tue, Mar 27, 2018 at 5:16 PM, Eric Blake <eblake@redhat.com> wrote:
> On 03/27/2018 10:01 AM, Marc-André Lureau wrote:
>>
>> On Tue, Mar 27, 2018 at 4:30 PM, Eric Blake <eblake@redhat.com> wrote:
>>>
>>> From: Peter Xu <peterx@redhat.com>
>>>
>>> When someone sends a command before QMP handshake, the error used to be
>>> like this:
>>>
>>> {"execute": "query-cpus"}
>>> {"error": {"class": "CommandNotFound", "desc":
>>> "Expecting capabilities negotiation with
>>> 'qmp_capabilities'"}}
>>>
>>> While after cf869d5317 it becomes:
>>>
>>> {"execute": "query-cpus"}
>>> {"error": {"class": "CommandNotFound", "desc":
>>> "The command query-cpus has not been found"}}
>>>
>>> Fix it back to the nicer one.
>>>
>>> Fixes: cf869d5317 ("qmp: support out-of-band (oob) execution",
>>> 2018-03-19)
>>> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> Signed-off-by: Peter Xu <peterx@redhat.com>
>>> Message-Id: <20180326063901.27425-2-peterx@redhat.com>
>>> Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>>
>> Reviewed-by (not x2 Reported-by ;)
>>
>
> Shoot. Want me to send a v2 pull request?
no need ;)
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 07/14] qapi: restrict allow-oob value to be "true"
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (5 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 06/14] qmp: fix qmp_capabilities error regression Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 08/14] tests: let qapi-schema tests detect oob Eric Blake
` (7 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu
From: Peter Xu <peterx@redhat.com>
It was missed in the first version of OOB series. We should check this
to make sure we throw the right error when fault value is passed in.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-5-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi/common.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 2c05e3c2845..3e14bc41f2c 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -872,7 +872,7 @@ def check_keys(expr_elem, meta, required, optional=[]):
raise QAPISemError(info,
"'%s' of %s '%s' should only use false value"
% (key, meta, name))
- if key == 'boxed' and value is not True:
+ if (key == 'boxed' or key == 'allow-oob') and value is not True:
raise QAPISemError(info,
"'%s' of %s '%s' should only use true value"
% (key, meta, name))
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 08/14] tests: let qapi-schema tests detect oob
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (6 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 07/14] qapi: restrict allow-oob value to be "true" Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 09/14] tests: add oob-test for qapi-schema Eric Blake
` (6 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Michael Roth
From: Peter Xu <peterx@redhat.com>
The allow_oob parameter was passed in but not used in tests. Now
reflect that in the tests, so we need to touch up other command testers
with that new change.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-6-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/qapi-schema/doc-good.out | 4 ++--
tests/qapi-schema/ident-with-escape.out | 2 +-
tests/qapi-schema/indented-expr.out | 4 ++--
tests/qapi-schema/qapi-schema-test.out | 18 +++++++++---------
tests/qapi-schema/test-qapi.py | 4 ++--
5 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 430b5a87db3..63058b1590a 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -28,9 +28,9 @@ object q_obj_cmd-arg
member arg2: str optional=True
member arg3: bool optional=False
command cmd q_obj_cmd-arg -> Object
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
command cmd-boxed Object -> None
- gen=True success_response=True boxed=True
+ gen=True success_response=True boxed=True oob=False
doc freeform
body=
= Section
diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out
index ee3b34e623e..82213aa51dd 100644
--- a/tests/qapi-schema/ident-with-escape.out
+++ b/tests/qapi-schema/ident-with-escape.out
@@ -5,4 +5,4 @@ module ident-with-escape.json
object q_obj_fooA-arg
member bar1: str optional=False
command fooA q_obj_fooA-arg -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index a79935e8c32..862678f8f4c 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -3,6 +3,6 @@ enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
prefix QTYPE
module indented-expr.json
command eins None -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
command zwei None -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 012e7fc06a5..4f43370017c 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -16,7 +16,7 @@ object Empty1
object Empty2
base Empty1
command user_def_cmd0 Empty2 -> Empty2
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
enum QEnumTwo ['value1', 'value2']
prefix QENUM_TWO
object UserDefOne
@@ -143,29 +143,29 @@ object UserDefNativeListUnion
case sizes: q_obj_sizeList-wrapper
case any: q_obj_anyList-wrapper
command user_def_cmd None -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
object q_obj_user_def_cmd1-arg
member ud1a: UserDefOne optional=False
command user_def_cmd1 q_obj_user_def_cmd1-arg -> None
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
object q_obj_user_def_cmd2-arg
member ud1a: UserDefOne optional=False
member ud1b: UserDefOne optional=True
command user_def_cmd2 q_obj_user_def_cmd2-arg -> UserDefTwo
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
object q_obj_guest-get-time-arg
member a: int optional=False
member b: int optional=True
command guest-get-time q_obj_guest-get-time-arg -> int
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
object q_obj_guest-sync-arg
member arg: any optional=False
command guest-sync q_obj_guest-sync-arg -> any
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
command boxed-struct UserDefZero -> None
- gen=True success_response=True boxed=True
+ gen=True success_response=True boxed=True oob=False
command boxed-union UserDefNativeListUnion -> None
- gen=True success_response=True boxed=True
+ gen=True success_response=True boxed=True oob=False
object UserDefOptions
member i64: intList optional=True
member u64: uint64List optional=True
@@ -229,4 +229,4 @@ object q_obj___org.qemu_x-command-arg
member c: __org.qemu_x-Union2 optional=False
member d: __org.qemu_x-Alt optional=False
command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1
- gen=True success_response=True boxed=False
+ gen=True success_response=True boxed=False oob=False
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 10e68b01d98..c1a144ba29a 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -45,8 +45,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
gen, success_response, boxed, allow_oob):
print('command %s %s -> %s' % \
(name, arg_type and arg_type.name, ret_type and ret_type.name))
- print(' gen=%s success_response=%s boxed=%s' % \
- (gen, success_response, boxed))
+ print(' gen=%s success_response=%s boxed=%s oob=%s' % \
+ (gen, success_response, boxed, allow_oob))
def visit_event(self, name, info, arg_type, boxed):
print('event %s %s' % (name, arg_type and arg_type.name))
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 09/14] tests: add oob-test for qapi-schema
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (7 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 08/14] tests: let qapi-schema tests detect oob Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 10/14] qmp: cleanup qmp queues properly Eric Blake
` (5 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Michael Roth
From: Peter Xu <peterx@redhat.com>
It simply tests the new OOB capability, and make sure the QAPISchema can
parse it correctly.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-7-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/test-qmp-cmds.c | 4 ++++
tests/Makefile.include | 1 +
tests/qapi-schema/oob-test.err | 1 +
tests/qapi-schema/oob-test.exit | 1 +
tests/qapi-schema/oob-test.json | 2 ++
tests/qapi-schema/oob-test.out | 0
tests/qapi-schema/qapi-schema-test.json | 3 +++
tests/qapi-schema/qapi-schema-test.out | 2 ++
8 files changed, 14 insertions(+)
create mode 100644 tests/qapi-schema/oob-test.err
create mode 100644 tests/qapi-schema/oob-test.exit
create mode 100644 tests/qapi-schema/oob-test.json
create mode 100644 tests/qapi-schema/oob-test.out
diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c
index 93fbbb1b733..db690cc5ae2 100644
--- a/tests/test-qmp-cmds.c
+++ b/tests/test-qmp-cmds.c
@@ -16,6 +16,10 @@ void qmp_user_def_cmd(Error **errp)
{
}
+void qmp_an_oob_command(Error **errp)
+{
+}
+
Empty2 *qmp_user_def_cmd0(Error **errp)
{
return g_new0(Empty2, 1);
diff --git a/tests/Makefile.include b/tests/Makefile.include
index eb218a9539d..3b9a5e31a2c 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -523,6 +523,7 @@ qapi-schema += missing-comma-object.json
qapi-schema += missing-type.json
qapi-schema += nested-struct-data.json
qapi-schema += non-objects.json
+qapi-schema += oob-test.json
qapi-schema += pragma-doc-required-crap.json
qapi-schema += pragma-extra-junk.json
qapi-schema += pragma-name-case-whitelist-crap.json
diff --git a/tests/qapi-schema/oob-test.err b/tests/qapi-schema/oob-test.err
new file mode 100644
index 00000000000..35b60f74800
--- /dev/null
+++ b/tests/qapi-schema/oob-test.err
@@ -0,0 +1 @@
+tests/qapi-schema/oob-test.json:2: 'allow-oob' of command 'oob-command-1' should only use true value
diff --git a/tests/qapi-schema/oob-test.exit b/tests/qapi-schema/oob-test.exit
new file mode 100644
index 00000000000..d00491fd7e5
--- /dev/null
+++ b/tests/qapi-schema/oob-test.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/oob-test.json b/tests/qapi-schema/oob-test.json
new file mode 100644
index 00000000000..da9635920fb
--- /dev/null
+++ b/tests/qapi-schema/oob-test.json
@@ -0,0 +1,2 @@
+# Check against oob illegal value
+{ 'command': 'oob-command-1', 'allow-oob': 'some-string' }
diff --git a/tests/qapi-schema/oob-test.out b/tests/qapi-schema/oob-test.out
new file mode 100644
index 00000000000..e69de29bb2d
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index c72dbd80509..06e30f452ea 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -139,6 +139,9 @@
{ 'command': 'boxed-struct', 'boxed': true, 'data': 'UserDefZero' }
{ 'command': 'boxed-union', 'data': 'UserDefNativeListUnion', 'boxed': true }
+# Smoke test on Out-Of-Band
+{ 'command': 'an-oob-command', 'allow-oob': true }
+
# For testing integer range flattening in opts-visitor. The following schema
# corresponds to the option format:
#
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 4f43370017c..467577d770b 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -166,6 +166,8 @@ command boxed-struct UserDefZero -> None
gen=True success_response=True boxed=True oob=False
command boxed-union UserDefNativeListUnion -> None
gen=True success_response=True boxed=True oob=False
+command an-oob-command None -> None
+ gen=True success_response=True boxed=False oob=True
object UserDefOptions
member i64: intList optional=True
member u64: uint64List optional=True
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 10/14] qmp: cleanup qmp queues properly
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (8 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 09/14] tests: add oob-test for qapi-schema Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 11/14] monitor: new parameter "x-oob" Eric Blake
` (4 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster, Dr. David Alan Gilbert
From: Peter Xu <peterx@redhat.com>
Marc-André Lureau reported that we can have this happen:
1. client1 connects, send command C1
2. client1 disconnects before getting response for C1
3. client2 connects, who might receive response of C1
However client2 should not receive remaining responses for client1.
Basically, we should clean up the request/response queue elements when:
- after a session is closed
- before destroying the queues
Some helpers are introduced to achieve that. We need to make sure we're
with the lock when operating on those queues. This also needed the
declaration of QMPRequest moved earlier.
Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-3-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[eblake: drop pointless qmp_response_free(), drop queue flush on connect
since a clean queue on disconnect is sufficient]
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
monitor.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 52 insertions(+), 19 deletions(-)
diff --git a/monitor.c b/monitor.c
index 3b1ef34711b..32d6114ac3a 100644
--- a/monitor.c
+++ b/monitor.c
@@ -234,6 +234,22 @@ static struct {
QEMUBH *qmp_respond_bh;
} mon_global;
+struct QMPRequest {
+ /* Owner of the request */
+ Monitor *mon;
+ /* "id" field of the request */
+ QObject *id;
+ /* Request object to be handled */
+ QObject *req;
+ /*
+ * Whether we need to resume the monitor afterward. This flag is
+ * used to emulate the old QMP server behavior that the current
+ * command must be completed before execution of the next one.
+ */
+ bool need_resume;
+};
+typedef struct QMPRequest QMPRequest;
+
/* QMP checker flags */
#define QMP_ACCEPT_UNKNOWNS 1
@@ -310,6 +326,38 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
}
}
+static void qmp_request_free(QMPRequest *req)
+{
+ qobject_decref(req->id);
+ qobject_decref(req->req);
+ g_free(req);
+}
+
+/* Must with the mon->qmp.qmp_queue_lock held */
+static void monitor_qmp_cleanup_req_queue_locked(Monitor *mon)
+{
+ while (!g_queue_is_empty(mon->qmp.qmp_requests)) {
+ qmp_request_free(g_queue_pop_head(mon->qmp.qmp_requests));
+ }
+}
+
+/* Must with the mon->qmp.qmp_queue_lock held */
+static void monitor_qmp_cleanup_resp_queue_locked(Monitor *mon)
+{
+ while (!g_queue_is_empty(mon->qmp.qmp_responses)) {
+ qobject_decref(g_queue_pop_head(mon->qmp.qmp_responses));
+ }
+}
+
+static void monitor_qmp_cleanup_queues(Monitor *mon)
+{
+ qemu_mutex_lock(&mon->qmp.qmp_queue_lock);
+ monitor_qmp_cleanup_req_queue_locked(mon);
+ monitor_qmp_cleanup_resp_queue_locked(mon);
+ qemu_mutex_unlock(&mon->qmp.qmp_queue_lock);
+}
+
+
static void monitor_flush_locked(Monitor *mon);
static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
@@ -701,6 +749,8 @@ static void monitor_data_destroy(Monitor *mon)
QDECREF(mon->outbuf);
qemu_mutex_destroy(&mon->out_lock);
qemu_mutex_destroy(&mon->qmp.qmp_queue_lock);
+ monitor_qmp_cleanup_req_queue_locked(mon);
+ monitor_qmp_cleanup_resp_queue_locked(mon);
g_queue_free(mon->qmp.qmp_requests);
g_queue_free(mon->qmp.qmp_responses);
}
@@ -4009,22 +4059,6 @@ static void monitor_qmp_respond(Monitor *mon, QObject *rsp,
qobject_decref(rsp);
}
-struct QMPRequest {
- /* Owner of the request */
- Monitor *mon;
- /* "id" field of the request */
- QObject *id;
- /* Request object to be handled */
- QObject *req;
- /*
- * Whether we need to resume the monitor afterward. This flag is
- * used to emulate the old QMP server behavior that the current
- * command must be completed before execution of the next one.
- */
- bool need_resume;
-};
-typedef struct QMPRequest QMPRequest;
-
/*
* Dispatch one single QMP request. The function will free the req_obj
* and objects inside it before return.
@@ -4191,9 +4225,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
qapi_event_send_command_dropped(id,
COMMAND_DROP_REASON_QUEUE_FULL,
&error_abort);
- qobject_decref(id);
- qobject_decref(req);
- g_free(req_obj);
+ qmp_request_free(req_obj);
return;
}
}
@@ -4335,6 +4367,7 @@ static void monitor_qmp_event(void *opaque, int event)
mon_refcount++;
break;
case CHR_EVENT_CLOSED:
+ monitor_qmp_cleanup_queues(mon);
json_message_parser_destroy(&mon->qmp.parser);
json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
mon_refcount--;
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 11/14] monitor: new parameter "x-oob"
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (9 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 10/14] qmp: cleanup qmp queues properly Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 12/14] tests: Add parameter to qtest_init_without_qmp_handshake Eric Blake
` (3 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Xu, Dr. David Alan Gilbert, Markus Armbruster,
Paolo Bonzini
From: Peter Xu <peterx@redhat.com>
Add new parameter to optionally enable Out-Of-Band for a QMP server.
An example command line:
./qemu-system-x86_64 -chardev stdio,id=char0 \
-mon chardev=char0,mode=control,x-oob=on
By default, Out-Of-Band is off.
It is not allowed if either MUX or non-QMP is detected, since
Out-Of-Band is currently only for QMP, and non-MUX chardev backends.
Note that the client STILL has to request 'oob' during qmp_capabilities;
in part because the x-oob command line option may disappear in the
future if we decide the capabilities negotiation is sufficient.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-4-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[eblake: enhance commit message]
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
include/monitor/monitor.h | 1 +
vl.c | 5 +++++
monitor.c | 22 ++++++++++++++++++++--
3 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 0cb0538a315..d6ab70cae23 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -13,6 +13,7 @@ extern Monitor *cur_mon;
#define MONITOR_USE_READLINE 0x02
#define MONITOR_USE_CONTROL 0x04
#define MONITOR_USE_PRETTY 0x08
+#define MONITOR_USE_OOB 0x10
bool monitor_cur_is_qmp(void);
diff --git a/vl.c b/vl.c
index c81cc866079..5fd01bd5f69 100644
--- a/vl.c
+++ b/vl.c
@@ -2404,6 +2404,11 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
if (qemu_opt_get_bool(opts, "pretty", 0))
flags |= MONITOR_USE_PRETTY;
+ /* OOB is off by default */
+ if (qemu_opt_get_bool(opts, "x-oob", 0)) {
+ flags |= MONITOR_USE_OOB;
+ }
+
chardev = qemu_opt_get(opts, "chardev");
chr = qemu_chr_find(chardev);
if (chr == NULL) {
diff --git a/monitor.c b/monitor.c
index 32d6114ac3a..51f4cf480f8 100644
--- a/monitor.c
+++ b/monitor.c
@@ -36,6 +36,7 @@
#include "net/slirp.h"
#include "chardev/char-fe.h"
#include "chardev/char-io.h"
+#include "chardev/char-mux.h"
#include "ui/qemu-spice.h"
#include "sysemu/numa.h"
#include "monitor/monitor.h"
@@ -4562,12 +4563,26 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
void monitor_init(Chardev *chr, int flags)
{
Monitor *mon = g_malloc(sizeof(*mon));
+ bool use_readline = flags & MONITOR_USE_READLINE;
+ bool use_oob = flags & MONITOR_USE_OOB;
- monitor_data_init(mon, false, false);
+ if (use_oob) {
+ if (CHARDEV_IS_MUX(chr)) {
+ error_report("Monitor Out-Of-Band is not supported with "
+ "MUX typed chardev backend");
+ exit(1);
+ }
+ if (use_readline) {
+ error_report("Monitor Out-Of-band is only supported by QMP");
+ exit(1);
+ }
+ }
+
+ monitor_data_init(mon, false, use_oob);
qemu_chr_fe_init(&mon->chr, chr, &error_abort);
mon->flags = flags;
- if (flags & MONITOR_USE_READLINE) {
+ if (use_readline) {
mon->rs = readline_init(monitor_readline_printf,
monitor_readline_flush,
mon,
@@ -4663,6 +4678,9 @@ QemuOptsList qemu_mon_opts = {
},{
.name = "pretty",
.type = QEMU_OPT_BOOL,
+ },{
+ .name = "x-oob",
+ .type = QEMU_OPT_BOOL,
},
{ /* end of list */ }
},
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 12/14] tests: Add parameter to qtest_init_without_qmp_handshake
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (10 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 11/14] monitor: new parameter "x-oob" Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 13/14] tests: qmp-test: add test for new "x-oob" Eric Blake
` (2 subsequent siblings)
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Markus Armbruster
Allow callers to choose whether to allow OOB support during a test;
for now, all existing callers pass false, but the next patch will
add a new caller. Also, rewrite the monitor setup to be generic
(using the -qmp shorthand is insufficient for honoring the parameter).
Based on an idea by Peter Xu, in <20180326063901.27425-8-peterx@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180327013620.1644387-4-eblake@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Acked-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
tests/libqtest.h | 7 +++++--
tests/libqtest.c | 10 ++++++----
tests/qmp-test.c | 2 +-
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/tests/libqtest.h b/tests/libqtest.h
index 811169453ab..cbe8df44730 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -56,11 +56,14 @@ QTestState *qtest_init(const char *extra_args);
/**
* qtest_init_without_qmp_handshake:
- * @extra_args: other arguments to pass to QEMU.
+ * @use_oob: true to have the server advertise OOB support
+ * @extra_args: other arguments to pass to QEMU. CAUTION: these
+ * arguments are subject to word splitting and shell evaluation.
*
* Returns: #QTestState instance.
*/
-QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
+QTestState *qtest_init_without_qmp_handshake(bool use_oob,
+ const char *extra_args);
/**
* qtest_quit:
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 200b2b9e92a..6f33a37667c 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -166,7 +166,8 @@ static const char *qtest_qemu_binary(void)
return qemu_bin;
}
-QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
+QTestState *qtest_init_without_qmp_handshake(bool use_oob,
+ const char *extra_args)
{
QTestState *s;
int sock, qmpsock, i;
@@ -199,12 +200,13 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
command = g_strdup_printf("exec %s "
"-qtest unix:%s,nowait "
"-qtest-log %s "
- "-qmp unix:%s,nowait "
+ "-chardev socket,path=%s,nowait,id=char0 "
+ "-mon chardev=char0,mode=control%s "
"-machine accel=qtest "
"-display none "
"%s", qemu_binary, socket_path,
getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null",
- qmp_socket_path,
+ qmp_socket_path, use_oob ? ",x-oob=on" : "",
extra_args ?: "");
execlp("/bin/sh", "sh", "-c", command, NULL);
exit(1);
@@ -239,7 +241,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
QTestState *qtest_init(const char *extra_args)
{
- QTestState *s = qtest_init_without_qmp_handshake(extra_args);
+ QTestState *s = qtest_init_without_qmp_handshake(false, extra_args);
/* Read the QMP greeting and then do the handshake */
qtest_qmp_discard_response(s, "");
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 8de99a4727e..2134d95db97 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -81,7 +81,7 @@ static void test_qmp_protocol(void)
QList *capabilities;
QTestState *qts;
- qts = qtest_init_without_qmp_handshake(common_args);
+ qts = qtest_init_without_qmp_handshake(false, common_args);
/* Test greeting */
resp = qtest_qmp_receive(qts);
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 13/14] tests: qmp-test: add test for new "x-oob"
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (11 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 12/14] tests: Add parameter to qtest_init_without_qmp_handshake Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 14:30 ` [Qemu-devel] [PULL 14/14] hmp.c: Revert hmp_info_cpus output format change Eric Blake
2018-03-27 15:23 ` [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Xu, Markus Armbruster
From: Peter Xu <peterx@redhat.com>
Test the new OOB capability. It's mostly the reverted OOB test
(see commit 4fd78ad7), but differs in that:
- It uses the new qtest_init_without_qmp_handshake() parameter to
create the monitor with "x-oob"
- Squashed the capability tests on greeting message
- Don't use qtest_global any more, instead use self-maintained
QTestState, which is the trend
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-9-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: rebase to qtest_init changes]
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
tests/qmp-test.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 82 insertions(+)
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 2134d95db97..772058fc4c4 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -135,6 +135,87 @@ static void test_qmp_protocol(void)
qtest_quit(qts);
}
+/* Tests for Out-Of-Band support. */
+static void test_qmp_oob(void)
+{
+ QTestState *qts;
+ QDict *resp, *q;
+ int acks = 0;
+ const QListEntry *entry;
+ QList *capabilities;
+ QString *qstr;
+ const char *cmd_id;
+
+ qts = qtest_init_without_qmp_handshake(true, common_args);
+
+ /* Check the greeting message. */
+ resp = qtest_qmp_receive(qts);
+ q = qdict_get_qdict(resp, "QMP");
+ g_assert(q);
+ capabilities = qdict_get_qlist(q, "capabilities");
+ g_assert(capabilities && !qlist_empty(capabilities));
+ entry = qlist_first(capabilities);
+ g_assert(entry);
+ qstr = qobject_to(QString, entry->value);
+ g_assert(qstr);
+ g_assert_cmpstr(qstring_get_str(qstr), ==, "oob");
+ QDECREF(resp);
+
+ /* Try a fake capability, it should fail. */
+ resp = qtest_qmp(qts,
+ "{ 'execute': 'qmp_capabilities', "
+ " 'arguments': { 'enable': [ 'cap-does-not-exist' ] } }");
+ g_assert(qdict_haskey(resp, "error"));
+ QDECREF(resp);
+
+ /* Now, enable OOB in current QMP session, it should succeed. */
+ resp = qtest_qmp(qts,
+ "{ 'execute': 'qmp_capabilities', "
+ " 'arguments': { 'enable': [ 'oob' ] } }");
+ g_assert(qdict_haskey(resp, "return"));
+ QDECREF(resp);
+
+ /*
+ * Try any command that does not support OOB but with OOB flag. We
+ * should get failure.
+ */
+ resp = qtest_qmp(qts,
+ "{ 'execute': 'query-cpus',"
+ " 'control': { 'run-oob': true } }");
+ g_assert(qdict_haskey(resp, "error"));
+ QDECREF(resp);
+
+ /*
+ * First send the "x-oob-test" command with lock=true and
+ * oob=false, it should hang the dispatcher and main thread;
+ * later, we send another lock=false with oob=true to continue
+ * that thread processing. Finally we should receive replies from
+ * both commands.
+ */
+ qtest_async_qmp(qts,
+ "{ 'execute': 'x-oob-test',"
+ " 'arguments': { 'lock': true }, "
+ " 'id': 'lock-cmd'}");
+ qtest_async_qmp(qts,
+ "{ 'execute': 'x-oob-test', "
+ " 'arguments': { 'lock': false }, "
+ " 'control': { 'run-oob': true }, "
+ " 'id': 'unlock-cmd' }");
+
+ /* Ignore all events. Wait for 2 acks */
+ while (acks < 2) {
+ resp = qtest_qmp_receive(qts);
+ cmd_id = qdict_get_str(resp, "id");
+ if (!g_strcmp0(cmd_id, "lock-cmd") ||
+ !g_strcmp0(cmd_id, "unlock-cmd")) {
+ acks++;
+ }
+ QDECREF(resp);
+ }
+
+ qtest_quit(qts);
+}
+
static int query_error_class(const char *cmd)
{
static struct {
@@ -319,6 +400,7 @@ int main(int argc, char *argv[])
g_test_init(&argc, &argv, NULL);
qtest_add_func("qmp/protocol", test_qmp_protocol);
+ qtest_add_func("qmp/oob", test_qmp_oob);
qmp_schema_init(&schema);
add_query_tests(&schema);
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PULL 14/14] hmp.c: Revert hmp_info_cpus output format change
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (12 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 13/14] tests: qmp-test: add test for new "x-oob" Eric Blake
@ 2018-03-27 14:30 ` Eric Blake
2018-03-27 15:23 ` [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 14:30 UTC (permalink / raw)
To: qemu-devel; +Cc: Satheesh Rajendran, Viktor Mihajlovski, Dr. David Alan Gilbert
From: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Commit 137b5cb6 refactored 'info cpus' output, changing
'thread_id' to 'thread-id'. While HMP is not a stable
interface, it is trivial to keep the spelling consistent
for test frameworks that have not yet updated to using QMP.
This patch just reverts back output format to 'thread_id'.
CC: Viktor Mihajlovski <mihajlov@linux.vnet.ibm.com>
Signed-off-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Message-Id: <20180327123800.28851-1-sathnaga@linux.vnet.ibm.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
[eblake: improve commit message]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
hmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hmp.c b/hmp.c
index 679467d85aa..a25c7bd9a81 100644
--- a/hmp.c
+++ b/hmp.c
@@ -381,7 +381,7 @@ void hmp_info_cpus(Monitor *mon, const QDict *qdict)
monitor_printf(mon, "%c CPU #%" PRId64 ":", active,
cpu->value->cpu_index);
- monitor_printf(mon, " thread-id=%" PRId64 "\n", cpu->value->thread_id);
+ monitor_printf(mon, " thread_id=%" PRId64 "\n", cpu->value->thread_id);
}
qapi_free_CpuInfoFastList(cpu_list);
--
2.14.3
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1
2018-03-27 14:30 [Qemu-devel] [PULL 00/14] QAPI changes for 2018-03-27, 2.12-rc1 Eric Blake
` (13 preceding siblings ...)
2018-03-27 14:30 ` [Qemu-devel] [PULL 14/14] hmp.c: Revert hmp_info_cpus output format change Eric Blake
@ 2018-03-27 15:23 ` Eric Blake
14 siblings, 0 replies; 19+ messages in thread
From: Eric Blake @ 2018-03-27 15:23 UTC (permalink / raw)
To: qemu-devel, Peter Maydell
On 03/27/2018 09:30 AM, Eric Blake wrote:
> The following changes since commit f58d9620aa4a514b1227074ff56eefd1334a6225:
>
> Merge remote-tracking branch 'remotes/rth/tags/pull-dt-20180326' into staging (2018-03-27 10:27:34 +0100)
>
> are available in the Git repository at:
>
> git://repo.or.cz/qemu/ericb.git tags/pull-qapi-2018-03-27
>
> for you to fetch changes up to 701a2432fea0a9a1741abf7a887fae26f5410970:
>
> hmp.c: Revert hmp_info_cpus output format change (2018-03-27 09:21:52 -0500)
v2 pull request sent to fix some last minute Reviewed-by tag corrections
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 19+ messages in thread