The __atomic primitives have been available since GCC 4.7 and provide
a richer interface for describing memory ordering requirements. As a
bonus by using the primitives instead of hand-rolled functions we can
use tools such as the ThreadSanitizer which need the use of well
defined APIs for its analysis.

If we have __ATOMIC defines we exclusively use the __atomic primitives
for all our atomic access. Otherwise we fall back to the mixture of
__sync and hand-rolled barrier cases.
Signed-off-by: NAlex Bennée <alex.bennee@linaro.org>
Message-Id: <1453976119-24372-4-git-send-email-alex.bennee@linaro.org>
[Use __ATOMIC_SEQ_CST for atomic_mb_read/atomic_mb_set on !POWER. - Paolo]
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Developers on 64-bit machines will often try to perform a
32-bit build of QEMU by running

  ./configure --extra-cflags="-m32"

Unfortunately if PKG_CONFIG_LIBDIR is not set to point to
the location of the 32-bit pkg-config files, then configure
will silently pick up the 64-bit pkg-config files and still
succeed.

This causes a problem for glib because it means QEMU will
be pulling in /usr/lib64/glib-2.0/include/glibconfig.h
instead of /usr/lib/glib-2.0/include/glibconfig.h

This causes problems because the 'gsize' type (defined as
'unsigned long') will no longer be fully compatible with
the 'size_t' type (defined as 'unsigned int'). Although
both are the same size, the compiler refuses to allow
casts from 'unsigned long *' to 'unsigned int *' as they
are different pointer types. This results in non-obvious
compiler errors when building QEMU eg

qga/commands-posix.c: In function ‘qmp_guest_set_user_password’:
qga/commands-posix.c:1912:55: error: passing argument 2 of ‘g_base64_decode’ from incompatible pointer type [-Werror=incompatible-pointer-types]
     rawpasswddata = (char *)g_base64_decode(password, &rawpasswdlen);
                                                            ^
In file included from /usr/include/glib-2.0/glib.h:35:0,
                 from qga/commands-posix.c:14:
/usr/include/glib-2.0/glib/gbase64.h:52:9: note: expected ‘gsize * {aka long unsigned int *}’ but argument is of type ‘size_t * {aka unsigned int *}’
 guchar *g_base64_decode         (const gchar  *text,
         ^
cc1: all warnings being treated as errors

To detect this problem, add a check to configure that
verifies that GLIB_SIZEOF_SIZE_T matches sizeof(size_t).
If this fails print a warning suggesting that the dev
probably needs to set PKG_CONFIG_LIBDIR.

On Fedora x86_64 it passes with any of:

 # ./configure
 # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m32"
 # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m64"

And fails with a mis-match

 # PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig ./configure --extra-cflags="-m32"
 # PKG_CONFIG_LIBDIR=/usr/lib/pkgconfig ./configure --extra-cflags="-m64"

ERROR: sizeof(size_t) doesn't match GLIB_SIZEOF_SIZE_T.
       You probably need to set PKG_CONFIG_LIBDIR
       to point to the right pkg-config files for your
       build target
Signed-off-by: NDaniel P. Berrange <berrange@redhat.com>
Message-Id: <1453885245-15562-1-git-send-email-berrange@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

If a process opens the slave pts device, writes data to it, then
immediately closes it, the data doesn't reliably get delivered to the
emulated serial port. This seems to be because a read of the master
pty device returns EIO on Linux if no process has the pts device open,
even when data is waiting "in the pipe".

A fix seems to be for QEMU to keep the pts file descriptor open until
the pty is closed, as per the below patch.
Signed-off-by: NAshley Jonathan <jonathan.ashley@altran.com>
Message-Id: <AC19797808C8D548ABDE0CA4A97AA30A30DEB409@XMB-DCFR-37.europe.corp.altran.com>
Reviewed-by: NMichael Tokarev <mjt@tls.msk.ru>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

Although accesses to ram_list.dirty_memory[] use atomics so multiple
threads can safely dirty the bitmap, the data structure is not fully
thread-safe yet.

This patch handles the RAM hotplug case where ram_list.dirty_memory[] is
grown.  ram_list.dirty_memory[] is change from a regular bitmap to an
RCU array of pointers to fixed-size bitmap blocks.  Threads can continue
accessing bitmap blocks while the array is being extended.  See the
comments in the code for an in-depth explanation of struct
DirtyMemoryBlocks.

I have tested that live migration with virtio-blk dataplane works.
Signed-off-by: NStefan Hajnoczi <stefanha@redhat.com>
Message-Id: <1453728801-5398-2-git-send-email-stefanha@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

This condition is true in the common case, so we can cut out the body of
the function.  In addition, this makes it easier for the compiler to do
at least partial inlining, even if it decides that fully inlining the
function is unreasonable.
Reviewed-by: NStefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: NPaolo Bonzini <pbonzini@redhat.com>

QAPI patches for 2016-02-09

# gpg: Signature made Tue 09 Feb 2016 10:55:51 GMT using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <armbru@redhat.com>"
# gpg:                 aka "Markus Armbruster <armbru@pond.sub.org>"

* remotes/armbru/tags/pull-qapi-2016-02-09: (31 commits)
  qapi: Add missing JSON files in build dependencies
  qapi: Fix compilation failure on MIPS and SPARC
  qmp: Don't abuse stack to track qmp-output root
  qmp: Fix reference-counting of qnull on empty output visit
  qapi: Drop unused error argument for list and implicit struct
  qapi: Tighten qmp_input_end_list()
  qapi: Drop unused 'kind' for struct/enum visit
  qapi: Swap 'name' in visit_* callbacks to match public API
  qom: Swap 'name' next to visitor in ObjectPropertyAccessor
  qapi: Swap visit_* arguments for consistent 'name' placement
  qom: Use typedef for Visitor
  qapi: Don't cast Enum* to int*
  qapi: Consolidate visitor small integer callbacks
  qapi: Make all visitors supply uint64 callbacks
  qapi: Prefer type_int64 over type_int in visitors
  qapi-visit: Kill unused visit_end_union()
  qapi: Track all failures between visit_start/stop
  qapi: Improve generated event use of qapi visitor
  balloon: Improve use of qapi visitor
  vl: Ensure qapi visitor properly ends struct visit
  ...
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>

Queued TCG patches

# gpg: Signature made Mon 08 Feb 2016 23:57:30 GMT using RSA key ID 4DD0279B
# gpg: Good signature from "Richard Henderson <rth7680@gmail.com>"
# gpg:                 aka "Richard Henderson <rth@redhat.com>"
# gpg:                 aka "Richard Henderson <rth@twiddle.net>"

* remotes/rth/tags/pull-tcg-20160209:
  tcg: Introduce temp_load
  tcg: Change temp_save argument to TCGTemp
  tcg: Change temp_sync argument to TCGTemp
  tcg: Change temp_dead argument to TCGTemp
  tcg: Change reg_to_temp to TCGTemp pointer
  tcg: Remove tcg_get_arg_str_i32/64
  tcg: More use of TCGReg where appropriate
  tcg: Work around clang bug wrt enum ranges
  tcg: Tidy temporary allocation
  tcg: Change ts->mem_reg to ts->mem_base
  tcg: Change tcg_global_mem_new_* to take a TCGv_ptr
  tcg: Remove lingering references to gen_opc_buf
  tcg: Respect highwater in tcg_out_tb_finalize
Signed-off-by: NPeter Maydell <peter.maydell@linaro.org>

Unify all of the places that realize a temporary into a register.
Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

A subsequent patch patch will change the type of REG from int
to enum TCGReg, which provokes the following bug in clang:

  https://llvm.org/bugs/show_bug.cgi?id=16154Signed-off-by: NRichard Henderson <rth@twiddle.net>

In particular, make sure the memory is memset before use.
Continues the increased use of TCGTemp pointers instead of
integer indices where appropriate.
Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Chain the temporaries together via pointers intstead of indices.
The mem_reg value is now mem_base->reg.  This will be important later.

This does require that the frame pointer have a global temporary
allocated for it.  This is simple bar the existing reserved_regs check.
Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Thus, use cpu_env as the parameter, not TCG_AREG0 directly.
Update all uses in the translators.
Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Three in comments and one in code in the stub tcg_liveness_analysis.
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Undo the workaround at b17a6d33.

If there are lots of memory operations in a TB, the slow path code
can exceed the highwater reservation.  Add a check within the loop.
Tested-by: NAurelien Jarno <aurelien@aurel32.net>
Reviewed-by: NAurelien Jarno <aurelien@aurel32.net>
Signed-off-by: NRichard Henderson <rth@twiddle.net>

Forgotten in commit 1dde0f48 (trace.json) and commit fafa4d50
(rocker.json).
Signed-off-by: NLluís Vilanova <vilanova@ac.upc.edu>
Message-Id: <145461055662.15201.2702170180078718114.stgit@localhost>
Reviewed-by: NEric Blake <eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Commit 86f4b687 broke compilation on MIPS and SPARC, which have a
preprocessor pollution of '#define mips 1' and '#define sparc 1',
respectively.  Treat it the same way as we do for the pollution with
'unix', so that QMP remains backwards compatible and only the C code
needs to use the alternative 'q_mips', 'q_sparc' spelling.

CC: James Hogan <james.hogan@imgtec.com>
CC: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: NEric Blake <eblake@redhat.com>
Tested-by: NJames Hogan <james.hogan@imgtec.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

The previous commit documented an inconsistency in how we are
using the stack of qmp-output-visitor.  Normally, pushing a
single top-level object puts the object on the stack twice:
once as the root, and once as the current container being
appended to; but popping that struct only pops once.  However,
qmp_ouput_add() was trying to either set up the added object
as the new root (works if you parse two top-level scalars in a
row: the second replaces the first as the root) or as a member
of the current container (works as long as you have an open
container on the stack; but if you have popped the first
top-level container, it then resolves to the root and still
tries to add into that existing container).

Fix the stupidity by not tracking two separate things in the
stack.  Drop the now-useless qmp_output_first() and
qmp_output_last() while at it.

Saved for a later patch: we still are rather sloppy in that
qmp_output_get_object() can be called in the middle of a parse,
rather than requiring that a visit is complete.
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-26-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Commit 6c2f9a15 ensured that we would not return NULL when the
caller used an output visitor but had nothing to visit. But
in doing so, it added a FIXME about a reference count leak
that could abort qemu in the (unlikely) case of SIZE_MAX such
visits (more plausible on 32-bit).  (Although that commit
suggested we might fix it in time for 2.5, we ran out of time;
fortunately, it is unlikely enough to bite that it was not
worth worrying about during the 2.5 release.)

This fixes things by documenting the internal contracts, and
explaining why the internal function can return NULL and only
the public facing interface needs to worry about qnull(),
thus avoiding over-referencing the qnull_ global object.

It does not, however, fix the stupidity of the stack mixing
up two separate pieces of information; add a FIXME to explain
that issue, which will be fixed shortly in a future patch.
Signed-off-by: NEric Blake <eblake@redhat.com>
Cc: qemu-stable@nongnu.org
Message-Id: <1454075341-13658-25-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

No backend was setting an error when ending the visit of a list or
implicit struct, or when moving to the next list node.  Make the
callers a bit easier to follow by making this a part of the contract,
and removing the errp argument - callers can then unconditionally end
an object as part of cleanup without having to think about whether a
second error is dominated by a first, because there is no second
error.

A later patch will then tackle the larger task of splitting
visit_end_struct(), which can indeed set an error.
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-24-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

The only way that qmp_input_pop() will set errp is if a dictionary
was the most recent thing pushed.  Since we don't have any
push(struct)/pop(list) or push(list)/pop(struct) mismatches (such
a mismatch is a programming bug), we therefore cannot set errp
inside qmp_input_end_list().  Make this obvious by
using &error_abort.  A later patch will then remove the errp
parameter of qmp_input_pop(), but that will first require the
larger task of splitting visit_end_struct().
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-23-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

visit_start_struct() and visit_type_enum() had a 'kind' argument
that was usually set to either the stringized version of the
corresponding qapi type name, or to NULL (although some clients
didn't even get that right).  But nothing ever used the argument.
It's even hard to argue that it would be useful in a debugger,
as a stack backtrace also tells which type is being visited.

Therefore, drop the 'kind' argument as dead.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-22-git-send-email-eblake@redhat.com>
[Harmless rebase mistake cleaned up]
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

As explained in the previous patches, matching argument order of
'name, &value' to JSON's "name":value makes sense.  However,
while the last two patches were easy with Coccinelle, I ended up
doing this one all by hand.  Now all the visitor callbacks match
the main interface.

The compiler is able to enforce that all clients match the changed
interface in visitor-impl.h, even where two pointers are being
swapped, because only one of the two pointers is const (if that
were not the case, then C's looseness on treating 'char *' like
'void *' would have made review a bit harder).
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-21-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Similar to the previous patch, it's nice to have all functions
in the tree that involve a visitor and a name for conversion to
or from QAPI to consistently stick the 'name' parameter next
to the Visitor parameter.

Done by manually changing include/qom/object.h and qom/object.c,
then running this Coccinelle script and touching up the fallout
(Coccinelle insisted on adding some trailing whitespace).

    @ rule1 @
    identifier fn;
    typedef Object, Visitor, Error;
    identifier obj, v, opaque, name, errp;
    @@
     void fn
    - (Object *obj, Visitor *v, void *opaque, const char *name,
    + (Object *obj, Visitor *v, const char *name, void *opaque,
       Error **errp) { ... }

    @@
    identifier rule1.fn;
    expression obj, v, opaque, name, errp;
    @@
     fn(obj, v,
    -   opaque, name,
    +   name, opaque,
        errp)
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-20-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

JSON uses "name":value, but many of our visitor interfaces were
called with visit_type_FOO(v, &value, name, errp).  This can be
a bit confusing to have to mentally swap the parameter order to
match JSON order.  It's particularly bad for visit_start_struct(),
where the 'name' parameter is smack in the middle of the
otherwise-related group of 'obj, kind, size' parameters! It's
time to do a global swap of the parameter ordering, so that the
'name' parameter is always immediately after the Visitor argument.

Additional reason in favor of the swap: the existing include/qjson.h
prefers listing 'name' first in json_prop_*(), and I have plans to
unify that file with the qapi visitors; listing 'name' first in
qapi will minimize churn to the (admittedly few) qjson.h clients.

Later patches will then fix docs, object.h, visitor-impl.h, and
those clients to match.

Done by first patching scripts/qapi*.py by hand to make generated
files do what I want, then by running the following Coccinelle
script to affect the rest of the code base:
 $ spatch --sp-file script `git grep -l '\bvisit_' -- '**/*.[ch]'`
I then had to apply some touchups (Coccinelle insisted on TAB
indentation in visitor.h, and botched the signature of
visit_type_enum() by rewriting 'const char *const strings[]' to
the syntactically invalid 'const char*const[] strings').  The
movement of parameters is sufficient to provoke compiler errors
if any callers were missed.

    // Part 1: Swap declaration order
    @@
    type TV, TErr, TObj, T1, T2;
    identifier OBJ, ARG1, ARG2;
    @@
     void visit_start_struct
    -(TV v, TObj OBJ, T1 ARG1, const char *name, T2 ARG2, TErr errp)
    +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
     { ... }

    @@
    type bool, TV, T1;
    identifier ARG1;
    @@
     bool visit_optional
    -(TV v, T1 ARG1, const char *name)
    +(TV v, const char *name, T1 ARG1)
     { ... }

    @@
    type TV, TErr, TObj, T1;
    identifier OBJ, ARG1;
    @@
     void visit_get_next_type
    -(TV v, TObj OBJ, T1 ARG1, const char *name, TErr errp)
    +(TV v, const char *name, TObj OBJ, T1 ARG1, TErr errp)
     { ... }

    @@
    type TV, TErr, TObj, T1, T2;
    identifier OBJ, ARG1, ARG2;
    @@
     void visit_type_enum
    -(TV v, TObj OBJ, T1 ARG1, T2 ARG2, const char *name, TErr errp)
    +(TV v, const char *name, TObj OBJ, T1 ARG1, T2 ARG2, TErr errp)
     { ... }

    @@
    type TV, TErr, TObj;
    identifier OBJ;
    identifier VISIT_TYPE =~ "^visit_type_";
    @@
     void VISIT_TYPE
    -(TV v, TObj OBJ, const char *name, TErr errp)
    +(TV v, const char *name, TObj OBJ, TErr errp)
     { ... }

    // Part 2: swap caller order
    @@
    expression V, NAME, OBJ, ARG1, ARG2, ERR;
    identifier VISIT_TYPE =~ "^visit_type_";
    @@
    (
    -visit_start_struct(V, OBJ, ARG1, NAME, ARG2, ERR)
    +visit_start_struct(V, NAME, OBJ, ARG1, ARG2, ERR)
    |
    -visit_optional(V, ARG1, NAME)
    +visit_optional(V, NAME, ARG1)
    |
    -visit_get_next_type(V, OBJ, ARG1, NAME, ERR)
    +visit_get_next_type(V, NAME, OBJ, ARG1, ERR)
    |
    -visit_type_enum(V, OBJ, ARG1, ARG2, NAME, ERR)
    +visit_type_enum(V, NAME, OBJ, ARG1, ARG2, ERR)
    |
    -VISIT_TYPE(V, OBJ, NAME, ERR)
    +VISIT_TYPE(V, NAME, OBJ, ERR)
    )
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-19-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

No need to repeat 'struct Visitor' when we already have it in
typedefs.h.  Omitting the redundant 'struct' also makes a later
patch easier to search for all object property callbacks that
are associated with a Visitor.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-18-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

C compilers are allowed to represent enums as a smaller type
than int, if all enum values fit in the smaller type.  There
are even compiler flags that force the use of this smaller
representation, although using them changes the ABI of a
binary. Therefore, our generated code for visit_type_ENUM()
(for all qapi enums) was wrong for casting Enum* to int* when
calling visit_type_enum().

It appears that no one has been using compiler ABI switches
for qemu, because if they had, we are potentially dereferencing
beyond bounds or even risking a SIGBUS on platforms where
unaligned pointer dereferencing is fatal.  But it is still
better to avoid the practice entirely, and just use the correct
types.

This matches the fix for alternate qapi types, done earlier in
commit 0426d53c "qapi: Simplify visiting of alternate types",
with generated code changing as:

| void visit_type_QType(Visitor *v, QType *obj, const char *name, Error **errp)
| {
|-    visit_type_enum(v, (int *)obj, QType_lookup, "QType", name, errp);
|+    int value = *obj;
|+    visit_type_enum(v, &value, QType_lookup, "QType", name, errp);
|+    *obj = value;
| }
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-17-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Commit 4e27e819 introduced optional visitor callbacks for all
sorts of int types, but no visitor has supplied any of the
callbacks for sizes less than 64 bits.  In other words, the
generic implementation based on using type_[u]int64() followed
by bounds-checking works just fine. In the interest of
simplicity, it's easier to make the visitor callback interface
not have to worry about the other sizes.

Adding some helper functions minimizes the boilerplate required
to correct FIXMEs added earlier with regards to questionable
reuse of errp, particularly now that we can guarantee from a
single file audit that value is unchanged if an error is set.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-16-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Our qapi visitor contract supports multiple integer visitors,
but left the type_uint64 visitor as optional (falling back on
type_int64); which in turn can lead to awkward behavior with
numbers larger than INT64_MAX (the user has to be aware of
twos complement, and deal with negatives).

This patch does not address the disparity in handling large
values as negatives.  It merely moves the fallback from uint64
to int64 from the visitor core to the visitors, where the issue
can actually be fixed, by implementing the missing type_uint64()
callbacks on top of the respective type_int64() callbacks, and
with a FIXME comment explaining why that's wrong.

With that done, we now have a type_uint64() callback in every
driver, so we can make it mandatory from the core.  And although
the type_int64() callback can cover the entire valid range of
type_uint{8,16,32} on valid user input, using type_uint64() to
avoid mixed signedness makes more sense.
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-15-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

The qapi builtin type 'int' is basically shorthand for the type
'int64'.  In fact, since no visitor was providing the optional
type_int64() callback, visit_type_int64() was just always falling
back to type_int(), cementing the equivalence between the types.

However, some visitors are providing a type_uint64() callback.
For purposes of code consistency, it is nicer if all visitors
use the paired type_int64/type_uint64 names rather than the
mismatched type_int/type_uint64.  So this patch just renames
the signed int callbacks in place, dropping the type_int()
callback as redundant, and a later patch will focus on the
unsigned int callbacks.

Add some FIXMEs to questionable reuse of errp in code touched
by the rename, while at it (the reuse works as long as the
callbacks don't modify value when setting an error, but it's not
a good example to set) - a later patch will then fix those.

No change in functionality here, although further cleanups are
in the pipeline.
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-14-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

The generated code can call visit_end_union() without having called
visit_start_union().  Example:

        if (!*obj) {
            goto out_obj;
        }
        visit_type_CpuInfoBase_fields(v, (CpuInfoBase **)obj, &err);
        if (err) {
            goto out_obj; // if we go from here...
        }
        if (!visit_start_union(v, !!(*obj)->u.data, &err) || err) {
            goto out_obj;
        }
        switch ((*obj)->arch) {
    [...]
        }
    out_obj:
        // ... then *obj is true, and ...
        error_propagate(errp, err);
        err = NULL;
        if (*obj) {
            // we end up here
            visit_end_union(v, !!(*obj)->u.data, &err);
        }
        error_propagate(errp, err);

Harmless only because no visitor implements end_union().  Clean it up
anyway, by deleting the function as useless.

Messed up since we have visit_end_union (commit cee2dedb).
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Message-Id: <1453902888-20457-3-git-send-email-armbru@redhat.com>
[expand scope of patch to delete rather than repair]
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-13-git-send-email-eblake@redhat.com>

Inside the generated code between visit_start_struct() and
visit_end_struct(), we were blindly setting the error into
the caller's errp parameter.  But a future patch to split
visit_end_struct() will require that we take action based
on whether an error has occurred, which requires us to track
all actions through a local err.  Rewrite the visits to be
more in line with the other generated calls.

Generated code changes look like:

|     visit_start_struct(v, (void **)obj, "Abort", name, sizeof(Abort), &err);
|-    if (!err) {
|-        if (*obj) {
|-            visit_type_Abort_fields(v, obj, errp);
|-        }
|-        visit_end_struct(v, &err);
|+    if (err) {
|+        goto out;
|     }
|+    if (!*obj) {
|+        goto out_obj;
|+    }
|+    visit_type_Abort_fields(v, obj, &err);
|+    error_propagate(errp, err);
|+    err = NULL;
|+out_obj:
|+    visit_end_struct(v, &err);
|+out:
|     error_propagate(errp, err);
| }
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-12-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

All other successful clients of visit_start_struct() were paired
with an unconditional visit_end_struct(); but the generated
code for events was relying on qmp_output_visitor_cleanup() to
work on an incomplete visit.  Alter the code to guarantee that
the struct is completed, which will make a future patch to
split visit_end_struct() easier to reason about.  While at it,
drop some assertions and comments that are not present in other
uses of the qmp output visitor, and pass NULL rather than "" as
the 'kind' parameter (matching most other uses where obj is NULL).

The changes to the generated code look like:

|     qmp = qmp_event_build_dict("DEVICE_TRAY_MOVED");
|
|     qov = qmp_output_visitor_new();
|-    g_assert(qov);
|-
|     v = qmp_output_get_visitor(qov);
|-    g_assert(v);
|
|-    /* Fake visit, as if all members are under a structure */
|-    visit_start_struct(v, NULL, "", "DEVICE_TRAY_MOVED", 0, &err);
|+    visit_start_struct(v, NULL, NULL, "DEVICE_TRAY_MOVED", 0, &err);
|     if (err) {
|         goto out;
|     }
|     visit_type_str(v, (char **)&device, "device", &err);
|     if (err) {
|-        goto out;
|+        goto out_obj;
|     }
|     visit_type_bool(v, &tray_open, "tray-open", &err);
|     if (err) {
|-        goto out;
|+        goto out_obj;
|     }
|-    visit_end_struct(v, &err);
|+out_obj:
|+    visit_end_struct(v, err ? NULL : &err);
|     if (err) {
|         goto out;
|     }
|
|     obj = qmp_output_get_qobject(qov);
|-    g_assert(obj != NULL);
|+    g_assert(obj);
|
|     qdict_put_obj(qmp, "data", obj);
|     emit(QAPI_EVENT_DEVICE_TRAY_MOVED, qmp, &err);

Note that the 'goto out_obj' with no intervening code before the
label, as well as the construct of 'err ? NULL : &err', are both
a bit unusual but also temporary; they get fixed in a later patch
that splits visit_end_struct() to drop its errp parameter by moving
some checking before the label.  But until that time, this was the
simplest way to avoid the appearance of passing a possibly-set
error to visit_end_struct(), even though actual code inspection
shows that visit_end_struct() for a QMP output visitor will never
set an error.
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-11-git-send-email-eblake@redhat.com>
[Commit message's code diff tweaked]
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Rework the control flow of balloon_stats_get_all() to make it
easier for a later patch to split visit_end_struct().  Also
switch to the uint64 visitor to match the data type.
Signed-off-by: NEric Blake <eblake@redhat.com>
Message-Id: <1454075341-13658-10-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Guarantee that visit_end_struct() is called if
visit_start_struct() succeeded.  This matches the behavior of
most other uses of visitors, and is a step towards the possibility
of a future patch that adds and enforces some tighter semantics to
the visitor interface (namely, cleanup of the visitor would no
longer have to mop up as many leftovers from an aborted partial
visit).

The change to code here matches the flow of hmp.c:hmp_object_add();
a later patch will then further simplify the cleanup logic of both
places by refactoring visit_end_struct() to not require a second
local error object.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <1454075341-13658-9-git-send-email-eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>