And use tcg pointer differencing functions as appropriate.
Reviewed-by: NPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

And use tcg pointer differencing functions as appropriate.
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>
Reviewed-by: NPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

And use tcg pointer differencing functions as appropriate.
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

And use tcg pointer differencing functions as appropriate.
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>
Reviewed-by: NPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

To be defined by the tcg backend based on the elemental unit of the ISA.
During the transition, allow TCG_TARGET_INSN_UNIT_SIZE to be undefined,
which allows us to default tcg_insn_unit to the current uint8_t.
Reviewed-by: NPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Reviewed-by: NPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

To avoid C undefined behaviour when patching generated code,
provide wrappers tcg_patch8/16/32/64 which use the usual memcpy
trick, and use them in the i386 backend.
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Avoid stores to unaligned addresses in TCG code generation, by using the
usual memcpy() approach. (Using bswap.h would drag a lot of QEMU baggage
into TCG, so it's simpler just to do direct memcpy() here.)
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

The code which patches x86 jump instructions assumes it can do an
unaligned write of a uint32_t. This is actually safe on x86, but it's
still undefined behaviour. We have infrastructure for doing efficient
unaligned accesses which doesn't engage in undefined behaviour, so
use it.

This is technically fractionally less efficient, at least with gcc 4.6;
instead of one instruction:
 7b2:   89 3e                   mov    %edi,(%rsi)
we get an extra spurious store to the stack slot:
 7b2:   89 7c 24 64             mov    %edi,0x64(%rsp)
 7b6:   89 3e                   mov    %edi,(%rsi)
Reviewed-by: NAlex Bennée <alex.bennee@linaro.org>
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

* remotes/qmp-unstable/queue/qmp: (38 commits)
  Revert "qapi: Clean up superfluous null check in qapi_dealloc_type_str()"
  qapi: Document optional arguments' backwards compatibility
  qmp: use valid JSON in transaction example
  qmp: Don't use error_is_set() to suppress additional errors
  dump: Drop pointless error_is_set(), DumpState member errp
  qemu-option: Clean up fragile use of error_is_set()
  qga: Drop superfluous error_is_set()
  qga: Clean up fragile use of error_is_set()
  qapi: Clean up fragile use of error_is_set()
  tests/qapi-schema: Drop superfluous error_is_set()
  qapi: Drop redundant, unclean error_is_set()
  hmp: Guard against misuse of hmp_handle_error()
  qga: Use return values instead of error_is_set(errp)
  error: Consistently name Error ** objects errp, and not err
  qmp: Consistently name Error ** objects errp, and not err
  qga: Consistently name Error ** objects errp, and not err
  qmp hmp: Consistently name Error * objects err, and not errp
  pci-assign: assigned_initfn(): set monitor error in common error handler
  pci-assign: propagate errors from assign_intx()
  pci-assign: propagate errors from assign_device()
  ...
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>

This reverts commit 25a70175.

Turns out the argument *can* be null: QEMU now segfaults if it
receives an invalid parameter via a qmp command instead of throwing an
error.

For example:
{ "execute": "blockdev-add",
     "arguments": { "options" : { "driver": "invalid-driver" } } }

CC: qemu-stable@nongnu.org
Signed-off-by: NPeter Lieven <pl@kamp.de>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Our example should use the correct quotes to match what someone
could actually pass over the wire.

* qmp-commands.hx: Use correct JSON quotes.
Signed-off-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Using error_is_set(errp) that way can sweep programming errors under
the carpet when we get called incorrectly with an error set.

encrypted_bdrv_it() does it, because there's no way to make
bdrv_iterate() break its loop.  Actually safe, because qmp_cont()
clears the error before the loop.  Clean it up anyway: replace
bdrv_iterate() by bdrv_next(), break the loop on error.

Replace both occurrences, for consistency.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

In qmp_dump_guest_memory(), the error must be clear on entry, and we
always bail out after setting it, directly or via dump_init().
Therefore, both error_is_set() are always false.  Drop them.

DumpState member errp is now write-only.  Drop it, too.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Using error_is_set(ERRP) to find out whether to bail out due to
previous error is either wrong, fragile, or unnecessarily opaque.
It's wrong when ERRP may be null, because errors go undetected when it
is.  It's fragile when proving ERRP non-null involves a non-local
argument.  Else, it's unnecessarily opaque (see commit 84d18f06).

The error_is_set(state->errp) in qemu_opts_from_qdict_1() is merely
fragile, because the callers never pass state argument with null
state->errp.

Make the code more robust and more obviously correct: test
*state->errp directly.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

acquire_privilege(), execute_async() and check_suspend_mode() do
nothing when called with an error set.  Callers shouldn't do that, and
no caller does.  Drop the superfluous tests.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMichael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque.  It's wrong when ERRP
may be null, because errors go undetected when it is.  It's fragile
when proving ERRP non-null involves a non-local argument.  Else, it's
unnecessarily opaque (see commit 84d18f06).

The error_is_set(errp) in the guest agent command handler functions
are merely fragile, because all chall chains (do_qmp_dispatch() via
the generated marshalling functions) pass a non-null errp argument.

Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMichael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Using error_is_set(ERRP) to find out whether a function failed is
either wrong, fragile, or unnecessarily opaque.  It's wrong when ERRP
may be null, because errors go undetected when it is.  It's fragile
when proving ERRP non-null involves a non-local argument.  Else, it's
unnecessarily opaque (see commit 84d18f06).

The error_is_set(errp) in do_qmp_dispatch() is merely fragile, because
the caller never passes a null errp argument.

Make the code more robust and more obviously correct: receive the
error in a local variable, then propagate it through the parameter.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMichael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

visit_type_TestStruct() does nothing when called with an error set.
Callers shouldn't do that, and no caller does.  Drop the superfluous
test.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMichael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

do_qmp_dispatch()'s test for qmp_dispatch_check_obj() failure examines
both the return value and the error object.  The latter part is
unclean; it works only when do_qmp_dispatch()'s caller passes a
non-null errp argument.  That's the case, but it's not locally
obvious.  Unclean.

Cleanup would be easy enough, but since the unclean code is also
redundant, let's just drop it.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMichael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Null errp argument makes no sense.  Assert it's not null, to make this
explicit, and guard against misuse.  All current callers pass non-null
errp.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Using error_is_set(errp) to check whether a function call failed is
fragile: it breaks when errp is null.  ga_get_fd_handle() and
guest_file_handle_add() don't return a useful value when they fail,
but that's just stupid.  Fix that, and check them instead.  As far
as I can tell, errp can't be null there, but this is more robust and
more obviously correct.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMichael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Among the callers, only assigned_initfn() should set the  monitor's stored
error. Other callers may run in contexts where the monitor's stored error
makes no sense. For example:

assigned_dev_pci_write_config()
  assigned_dev_update_msix()
    assign_intx()
Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Also, change the return type to "void"; the function is static (with a
sole caller) and the negative errno values are not distinguished from each
other.
Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

The return type is also changed from "int" to "void", because it was used
in a success vs. failure sense only (the caller didn't distinguish error
codes from each other, and even assigned_dev_register_msix_mmio() masked
mmap()'s errno values with a common -EFAULT).
Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

assigned_initfn()
  get_real_device()
    read()
Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Propagate any errors while adding PCI capabilities to
assigned_device_pci_cap_init(). We'll continue the propagation upwards
when assigned_device_pci_cap_init() becomes a leaf itself (when none of
its callees will report errors internally any longer when detecting and
returning them).
Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

... and rebase pci_add_capability() to it.
Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

Rename check_irqchip_in_kernel() to verify_irqchip_in_kernel(), so that
the name reflects our expectation better. Rather than returning a bool,
make it do nothing or set an Error.
Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

get_real_id() has two thin wrappers (and no other callers),
get_real_vendor_id() and get_real_device_id(); it's easiest to convert
them in one fell swoop.
Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>

This allows us to report the entire error with one error_report() call,
easing future error propagation.
Signed-off-by: NLaszlo Ersek <lersek@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NLuiz Capitulino <lcapitulino@redhat.com>