We need to set cs->halted to 1 before calling ppc_set_compat. The reason
is that ppc_set_compat kicks up the new thread created to manage the
hotplugged KVM virtual CPU and the code drives directly to KVM_RUN
ioctl. When cs->halted is 1, the code:
int kvm_cpu_exec(CPUState *cpu)
...
if (kvm_arch_process_async_events(cpu)) {
atomic_set(&cpu->exit_request, 0);
return EXCP_HLT;
}
...
returns before it reaches KVM_RUN, giving time to the main thread to
finish its job. Otherwise we can fall in a deadlock because the KVM
thread will issue the KVM_RUN ioctl while the main thread is setting up
KVM registers. Depending on how these jobs are scheduled we'll end up
freezing QEMU.
The following output shows kvm_vcpu_ioctl sleeping because it cannot get
the mutex and never will.
PS: kvm_vcpu_ioctl was triggered kvm_set_one_reg - compat_pvr.
STATE: TASK_UNINTERRUPTIBLE|TASK_WAKEKILL
PID: 61564 TASK: c000003e981e0780 CPU: 48 COMMAND: "qemu-system-ppc"
#0 [c000003e982679a0] __schedule at c000000000b10a44
#1 [c000003e98267a60] schedule at c000000000b113a8
#2 [c000003e98267a90] schedule_preempt_disabled at c000000000b11910
#3 [c000003e98267ab0] __mutex_lock at c000000000b132ec
#4 [c000003e98267bc0] kvm_vcpu_ioctl at c00800000ea03140 [kvm]
#5 [c000003e98267d20] do_vfs_ioctl at c000000000407d30
#6 [c000003e98267dc0] ksys_ioctl at c000000000408674
#7 [c000003e98267e10] sys_ioctl at c0000000004086f8
#8 [c000003e98267e30] system_call at c00000000000b488
crash> struct -x kvm.vcpus 0xc000003da0000000
vcpus = {0xc000003db4880000, 0xc000003d52b80000, 0xc0000039e9c80000, 0xc000003d0e200000, 0xc000003d58280000, 0x0, 0x0, ...}
crash> struct -x kvm_vcpu.mutex.owner 0xc000003d58280000
mutex.owner = {
counter = 0xc000003a23a5c881 <- flag 1: waiters
},
crash> bt 0xc000003a23a5c880
PID: 61579 TASK: c000003a23a5c880 CPU: 9 COMMAND: "CPU 4/KVM"
(active)
crash> struct -x kvm_vcpu.mutex.wait_list 0xc000003d58280000
mutex.wait_list = {
next = 0xc000003e98267b10,
prev = 0xc000003e98267b10
},
crash> struct -x mutex_waiter.task 0xc000003e98267b10
task = 0xc000003e981e0780
The following command-line was used to reproduce the problem (note: gdb
and trace can change the results).
$ qemu-ppc/build/ppc64-softmmu/qemu-system-ppc64 -cpu host \
-enable-kvm -m 4096 \
-smp 4,maxcpus=8,sockets=1,cores=2,threads=4 \
-display none -nographic \
-drive file=disk1.qcow2,format=qcow2
...
(qemu) device_add host-spapr-cpu-core,core-id=4
[no interaction is possible after it, only SIGKILL to take the terminal
back]
Signed-off-by: Jose Ricardo Ziviani <joserz@linux.ibm.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
spapr_init_cpus() currently creates spapr-cpu-core objects via
object_new() and setting their realized property to true. This leaves
their reference count at two, because object_new() adds an initial
reference and the realization attaches them to a default parent object
which also increments the reference count.
This causes a problem if one of these cores is hot unplugged: no
delete event is generated for it because it's reference count doesn't
reach zero when it is detached from it's parent.
Correct this by adding a call to object_unref() in spapr_init_cpus().
Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This enables the correct generation of bootdevice fw paths for in-built IDE
and virtio-pci-blk devices suitable for OpenBIOS.
Note we also set the MachineClass ignore_boot_device_suffixes property to true
since an additional disk node should not be added except for virtio devices.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This enables the correct generation of bootdevice fw paths for in-built IDE
and virtio-pci-blk devices suitable for OpenBIOS.
Note we also set the MachineClass ignore_boot_device_suffixes property to true
since an additional disk node should not be added except for virtio devices.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
This contains the offset of the IDE controller within the macio address space
and is required to allow the address to be included within the fw path.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
As the in-built IDE controller is attached to the macio bus then we should also
model this the same in QEMU to aid fw path generation.
Note that all existing macio devices are moved onto the new macio bus so that
the qdev tree accurately reflects the real hardware.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Commit 2c88b098e7 added a call to SPAPR_MACHINE_GET_CLASS(spapr) in
spapr_phb_realize() before we check spapr isn't NULL. This causes QEMU
to crash when starting a non-pseries machine with a sPAPR PHB.
This could be fixed by setting the smc variable after the null check,
but it seems more explicit to use a ternary operator to skip the call
to SPAPR_MACHINE_GET_CLASS() if spapr is NULL, since spapr_phb_realize()
will return immediately in this case.
This was reported by Coverity (CID 1395170 and 1395183).
Fixes: 2c88b098e7
Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Introduced in 04d595b300 ("spapr: do not use CPU_FOREACH_REVERSE",
2018-08-23)
Fixes: CID1395181
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Emilio G. Cota <cota@braap.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
There is no known available OS for ppc around anymore that uses page
sizes below 4k, so it does not make much sense that we keep wasting
our time on building and testing the ppcemb-softmmu target. It has
been deprecated since two releases, and nobody complained, so let's
remove this now.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
-----BEGIN PGP SIGNATURE-----
iQIcBAABCAAGBQJbg8tbAAoJEPSH7xhYctcjU/UQAMAuCBW+qoAdHp53i8L7Eh7g
GuEN1TNG0njQMbfAqlYEVCUnnyR3X52/ag0/zboS/pb77WAAAyxKtHK0dKYKVtEs
89377YXwNA/I0boiRL9JXzzcNwZWQYlTuaKzWYBPC0UvEJ1e1yxbTgCoMKzNOlg/
69OfWErKIJgS179fFFSnO1PvIkCZjRuGyHsgl2WfHfMjUALASuQLIDo6gj+wTzjO
O228X075r9+O5ahdkl1de7pvx0OZ9QE1gXSVukCen4Wk63HKsV0Wn2ixXAFdNmUQ
dxl5JGSexmJALQhRmn5KLd/6ULYMQyoTrnvvTQeBPPVNJsCS2h0FJokRKqcXeq4C
dw9ZIc2JPydVwo5DCrHFTOg+wYxO0AyyqI9OVCYOAQG1y4KUnOC3YBlqyg0Cu3wU
fCQ+WIU+MzvLYObmW4EYnCJDf+8wxWRc3rIt7bqDDO0on6VdgV7ifSz6E9O3mlOP
GJ+O8KUizZXYCETFmH+bk24kBsANBtDJveVjG0lc/a12F26Y3FWoQINlaSeyyeYy
rPdTs7D1W52/Quh5WoR+S4E5grpdNvWBGdBgJmwhORKLXTGmrDGQeGt9M1Mb3Ksk
Wx9qr/eBSAx1LvS7BnJMJ4Cl2UZeIgJEwMiop/Lk4XbbD3M6j7x5tm+4PX+9H86j
+cDcnyKSyLCbQQP6Ln8I
=9l1t
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/juanquintela/tags/check/20180827' into staging
check/next for 20180827
# gpg: Signature made Mon 27 Aug 2018 10:58:51 BST
# gpg: using RSA key F487EF185872D723
# gpg: Good signature from "Juan Quintela <quintela@redhat.com>"
# gpg: aka "Juan Quintela <quintela@trasno.org>"
# Primary key fingerprint: 1899 FF8E DEBF 58CC EE03 4B82 F487 EF18 5872 D723
* remotes/juanquintela/tags/check/20180827:
check: Move wdt_ib700 test to common
check: Move endianess test to common
check: Move VMXNET3 test to common
check: Only test boot-serial when sga is compiled in
check: Only test ivshm when it is compiled in
x86_64-softmmu: Configuration is identical to i386-softmmu
check: Only test usb-xhci-nec when it is compiled in
check: Only test isa-testdev when it is compiled in
configure: We don't want to clean configuration files
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Replace all the trace_vtd_err_*() hooks with the new error_report_once()
since they are similar to trace_vtd_err() - dumping the first error
would be mostly enough, then we have them on by default too.
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180815095328.32414-4-peterx@redhat.com>
[Use "%x" instead of "%" PRIx16 to print uint16_t, whitespace tidied up]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Replace existing trace_vtd_err() with error_report_once() then stderr
will capture something if any of the error happens, meanwhile we don't
suffer from any DDOS. Then remove the trace point. Since at it,
provide more information where proper (now we can pass parameters into
the report function).
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180815095328.32414-3-peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
[Two format strings fixed, whitespace tidied up]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
It is already protected by CONFIG_ISA_TESTDEV in all architectures.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
We protect it with CONFIG_VMXNET3_PCI now, so no need to also put it
on i386.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
This is only for x86* architecture.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
If we ever changed that, just make the things that are different
explicit.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Once there, untangle endianness-test and boot-serial-test.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
boot-serial-test don't depend on isa-testdev. Thanks Thomas.
If you don't want to compile everything, you configure
config-devices.mak. And then make clean remove it, and make will
create a default one without your configuration. Fix it by not
removing it on clean target. Remove it instead on distclean.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
---
Remove it instead on distclean.
vhost-user-gpu will share the same code to open a DRM node.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20180713130916.4153-20-marcandre.lureau@redhat.com>
[ kraxel: buildfix: util/drm.o must be CONFIG_OPENGL not CONFIG_LINUX ]
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
There are many error_report()s that can be used in frequently called
functions, especially on IO paths. That can be unideal in that
malicious guest can try to trigger the error tons of time which might
use up the log space on the host (e.g., libvirt can capture the stderr
of QEMU and put it persistently onto disk). In VT-d emulation code, we
have trace_vtd_error() tracer. AFAIU all those places can be replaced
by something like error_report() but trace points are mostly used to
avoid the DDOS attack that mentioned above. However using trace points
mean that errors are not dumped if trace not enabled.
It's not a big deal in most modern server managements since we have
things like logrotate to maintain the logs and make sure the quota is
expected. However it'll still be nice that we just provide another way
to restrict message generations. In most cases, this kind of
error_report()s will only provide valid information on the first message
sent, and all the rest of similar messages will be mostly talking about
the same thing. This patch introduces *_report_once() helpers to allow
a message to be dumped only once during one QEMU process's life cycle.
It will make sure: (1) it's on by deffault, so we can even get something
without turning the trace on and reproducing, and (2) it won't be
affected by DDOS attack.
To implement it, I stole the printk_once() macro from Linux.
CC: Eric Blake <eblake@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180815095328.32414-2-peterx@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Whitespace adjusted, comments improved]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1
iQEcBAABAgAGBQJbgCm7AAoJENSXKoln91plhJYH/jRqbCaUd04nGuyjOaYUajTL
brz3JD0XN2jD6NnDYUpuiNzawSojNzSklMA0u9AJiG+cpNK+gqW4fX+CYeX7ApjK
99+SXSejxnK3IJUNblQDD/hdCv9Dc1r12R7c80lm+aqJwi4C8hfULTbfrse/QdyA
KIHKl+c3uaWTPG2qC3mpPW/QS+IRPgRRwF/7GILuiagNmMcXyuMd2fQuePnf1rvD
ztTdtNJ0zfdFK1jlLa7D9Xe36RpS1uBinF429dNwXWM/+i1shvxc3Enzb4qEQNYe
ZeVxTomP/nO1elLZYdVUwdQYr6vmnvb1/mtTT6nq0NvHeLGjMZMqOBWGuAtdXoo=
=a8Jv
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-aug-2018' into staging
MIPS queue August 2018 v6
# gpg: Signature made Fri 24 Aug 2018 16:52:27 BST
# gpg: using RSA key D4972A8967F75A65
# gpg: Good signature from "Aleksandar Markovic <amarkovic@wavecomp.com>"
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg: There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 8526 FBF1 5DA3 811F 4A01 DD75 D497 2A89 67F7 5A65
* remotes/amarkovic/tags/mips-queue-aug-2018: (45 commits)
target/mips: Add definition of nanoMIPS I7200 CPU
mips_malta: Fix semihosting argument passing for nanoMIPS bare metal
mips_malta: Add setting up GT64120 BARs to the nanoMIPS bootloader
mips_malta: Add basic nanoMIPS boot code for Malta board
elf: Don't check FCR31_NAN2008 bit for nanoMIPS
elf: On elf loading, treat both EM_MIPS and EM_NANOMIPS as legal for MIPS
elf: Relax MIPS' elf_check_arch() to accept EM_NANOMIPS too
elf: Add EM_NANOMIPS value as a valid one for e_machine field
target/mips: Fix ERET/ERETNC behavior related to ADEL exception
target/mips: Add updating BadInstr and BadInstrX for nanoMIPS
target/mips: Add availability control via bit NMS
target/mips: Add emulation of DSP ASE for nanoMIPS - part 6
target/mips: Add emulation of DSP ASE for nanoMIPS - part 5
target/mips: Add emulation of DSP ASE for nanoMIPS - part 4
target/mips: Add emulation of DSP ASE for nanoMIPS - part 3
target/mips: Add emulation of DSP ASE for nanoMIPS - part 2
target/mips: Add emulation of DSP ASE for nanoMIPS - part 1
target/mips: Implement MT ASE support for nanoMIPS
target/mips: Fix pre-nanoMIPS MT ASE instructions availability control
target/mips: Add emulation of nanoMIPS 32-bit branch instructions
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
The previous commit makes JSON strings containing '%' awkward to
express in templates: you'd have to mask the '%' with an Unicode
escape \u0025. No template currently contains such JSON strings.
Support the printf conversion specification %% in JSON strings as a
convenience anyway, because it's trivially easy to do.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-58-armbru@redhat.com>
The JSON parser optionally supports interpolation. This is used to
build QObjects by parsing string templates. The templates are C
literals, so parse errors (such as invalid interpolation
specifications) are actually programming errors. Consequently, the
functions providing parsing with interpolation
(qobject_from_jsonf_nofail(), qobject_from_vjsonf_nofail(),
qdict_from_jsonf_nofail(), qdict_from_vjsonf_nofail()) pass
&error_abort to the parser.
However, there's another, more dangerous kind of programming error:
since we use va_arg() to get the value to interpolate, behavior is
undefined when the variable argument isn't consistent with the
interpolation specification.
The same problem exists with printf()-like functions, and the solution
is to have the compiler check consistency. This is what
GCC_FMT_ATTR() is about.
To enable this type checking for interpolation as well, we carefully
chose our interpolation specifications to match printf conversion
specifications, and decorate functions parsing templates with
GCC_FMT_ATTR().
Note that this only protects against undefined behavior due to type
errors. It can't protect against use of invalid interpolation
specifications that happen to be valid printf conversion
specifications.
However, there's still a gaping hole in the type checking: GCC
recognizes '%' as start of printf conversion specification anywhere in
the template, but the parser recognizes it only outside JSON strings.
For instance, if someone were to pass a "{ '%s': %d }" template, GCC
would require a char * and an int argument, but the parser would
va_arg() only an int argument, resulting in undefined behavior.
Avoid undefined behavior by catching the programming error at run
time: have the parser recognize and reject '%' in JSON strings.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-57-armbru@redhat.com>
The recursive descent parser passes along a pointer to
JSONParserContext. It additionally passes a pointer to interpolation
state (a va_alist *) as needed to reach its consumer
parse_interpolation().
Stuffing the latter pointer into JSONParserContext saves us the
trouble of passing it along, so do that.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-56-armbru@redhat.com>
test_after_failed_device_add() does this:
response = qmp("{'execute': 'device_add',"
" 'arguments': {"
" 'driver': 'virtio-blk-%s',"
" 'drive': 'drive0'"
"}}", qvirtio_get_dev_type());
Wrong. An interpolation specification must be a JSON token, it
doesn't work within JSON string tokens. The code above doesn't use
the value of qvirtio_get_dev_type(), and sends arguments
{"driver": "virtio-blk-%s", "drive": "drive0"}}
The command fails because there is no driver named "virtio-blk-%".
Harmless, since the test wants the command to fail. Screwed up in
commit 2f84a92ec6.
Fix the obvious way. The command now fails because the drive is
empty, like it did before commit 2f84a92ec6.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-55-armbru@redhat.com>
The JSON parser has three public headers, json-lexer.h, json-parser.h,
json-streamer.h. They all contain stuff that is of no interest
outside qobject/json-*.c.
Collect the public interface in include/qapi/qmp/json-parser.h, and
everything else in qobject/json-parser-int.h.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-54-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-53-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-52-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-51-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-50-armbru@redhat.com>
Token count and size limits exist to guard against excessive heap
usage. We check them only after we created the token on the heap.
That's assigning a cowboy to the barn to lasso the horse after it has
bolted. Close the barn door instead: check before we create the
token.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-49-armbru@redhat.com>
The last case where qobject_from_json() & friends return null without
setting an error is empty or blank input. Callers:
* block.c's parse_json_protocol() reports "Could not parse the JSON
options". It's marked as a work-around, because it also covered
actual bugs, but they got fixed in the previous few commits.
* qobject_input_visitor_new_str() reports "JSON parse error". Also
marked as work-around. The recent fixes have made this unreachable,
because it currently gets called only for input starting with '{'.
* check-qjson.c's empty_input() and blank_input() demonstrate the
behavior.
* The other callers are not affected since they only pass input with
exactly one JSON value or, in the case of negative tests, one error.
Fail with "Expecting a JSON value" instead of returning null, and
simplify callers.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-48-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-47-armbru@redhat.com>
json_message_process_token() accumulates tokens until it got the
sequence of tokens that comprise a single JSON value (it counts curly
braces and square brackets to decide). It feeds those token sequences
to json_parser_parse(). If a non-empty sequence of tokens remains at
the end of the parse, it's silently ignored. check-qjson.c cases
unterminated_array(), unterminated_array_comma(), unterminated_dict(),
unterminated_dict_comma() demonstrate this bug.
Fix as follows. Introduce a JSON_END_OF_INPUT token. When the
streamer receives it, it feeds the accumulated tokens to
json_parser_parse().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-46-armbru@redhat.com>
json-parser.c carefully reports end of input like this:
token = parser_context_pop_token(ctxt);
if (token == NULL) {
parse_error(ctxt, NULL, "premature EOI");
goto out;
}
Except parser_context_pop_token() can't return null, it fails its
assertion instead. Same for parser_context_peek_token(). Broken in
commit 65c0f1e955, and faithfully preserved in commit 95385fe9ac.
Only a latent bug, because the streamer throws away any input that
could trigger it.
Drop the assertions, so we can fix the streamer in the next commit.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-45-armbru@redhat.com>
qobject_from_json() & friends use the consume_json() callback to
receive either a value or an error from the parser.
When they are fed a string that contains more than either one JSON
value or one JSON syntax error, consume_json() gets called multiple
times.
When the last call receives a value, qobject_from_json() returns that
value. Any other values are leaked.
When any call receives an error, qobject_from_json() sets the first
error received. Any other errors are thrown away.
When values follow errors, qobject_from_json() returns both a value
and sets an error. That's bad. Impact:
* block.c's parse_json_protocol() ignores and leaks the value. It's
used to to parse pseudo-filenames starting with "json:". The
pseudo-filenames can come from the user or from image meta-data such
as a QCOW2 image's backing file name.
* vl.c's parse_display_qapi() ignores and leaks the error. It's used
to parse the argument of command line option -display.
* vl.c's main() case QEMU_OPTION_blockdev ignores the error and leaves
it in @err. main() will then pass a pointer to a non-null Error *
to net_init_clients(), which is forbidden. It can lead to assertion
failure or other misbehavior.
* check-qjson.c's multiple_values() demonstrates the badness.
* The other callers are not affected since they only pass strings with
exactly one JSON value or, in the case of negative tests, one
error.
The impact on the _nofail() functions is relatively harmless. They
abort when any call receives an error. Else they return the last
value, and leak the others, if any.
Fix consume_json() as follows. On the first call, save value and
error as before. On subsequent calls, if any, don't save them. If
the first call saved a value, the next call, if any, replaces the
value by an "Expecting at most one JSON value" error. Take care not
to leak values or errors that aren't saved.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-44-armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-43-armbru@redhat.com>
Support for %I64d got added in commit 2c0d4b36e7 "json: fix PRId64 on
Win32". We had to hard-code I64d because we used the lexer's finite
state machine to check interpolations. No more, so clean this up.
Additional conversion specifications would be easy enough to implement
when needed.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180823164025.12553-42-armbru@redhat.com>
