[Qemu-commits] [qemu/qemu] 983f52: qapi-visit: Add visitor.type classification

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 983f52d4b3f86fb9dc9f8b142132feb5a8723016
      
https://github.com/qemu/qemu/commit/983f52d4b3f86fb9dc9f8b142132feb5a8723016
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/opts-visitor.c
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qmp-input-visitor.c
    M qapi/qmp-output-visitor.c
    M qapi/string-input-visitor.c
    M qapi/string-output-visitor.c

  Log Message:
  -----------
  qapi-visit: Add visitor.type classification

We have three classes of QAPI visitors: input, output, and dealloc.
Currently, all implementations of these visitors have one thing in
common based on their visitor type: the implementation used for the
visit_type_enum() callback.  But since we plan to add more such
common behavior, in relation to documenting and further refining
the semantics, it makes more sense to have the visitor
implementations advertise which class they belong to, so the common
qapi-visit-core code can use that information in multiple places.

A later patch will better document the types of visitors directly
in visitor.h.

For this patch, knowing the class of a visitor implementation lets
us make input_type_enum() and output_type_enum() become static
functions, by replacing the callback function Visitor.type_enum()
with the simpler enum member Visitor.type.  Share a common
assertion in qapi-visit-core as part of the refactoring.

Move comments in opts-visitor.c to match the refactored layout.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: e58d695e6c3a5cfa0aa2fc91b87ade017ef28b05
      
https://github.com/qemu/qemu/commit/e58d695e6c3a5cfa0aa2fc91b87ade017ef28b05
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qapi/opts-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qmp-input-visitor.c
    M qapi/string-input-visitor.c
    M tests/test-qmp-input-strict.c

  Log Message:
  -----------
  qapi: Guarantee NULL obj on input visitor callback error

Our existing input visitors were not very consistent on errors in a
function taking 'TYPE **obj'.  These are start_struct(),
start_alternate(), type_str(), and type_any().  next_list() is
similar, but can't fail (see commit 08f9541).  While all of them set
'*obj' to allocated storage on success, it was not obvious whether
'*obj' was guaranteed safe on failure, or whether it was left
uninitialized.  But a future patch wants to guarantee that
visit_type_FOO() does not leak a partially-constructed obj back to
the caller; it is easier to implement this if we can reliably state
that input visitors assign '*obj' regardless of success or failure,
and that on failure *obj is NULL.  Add assertions to enforce
consistency in the final setting of err vs. *obj.

The opts-visitor start_struct() doesn't set an error, but it
also was doing a weird check for 0 size; all callers pass in
non-zero size if obj is non-NULL.

The testsuite has at least one spot where we no longer need
to pre-initialize a variable prior to a visit; valgrind confirms
that the test is still fine with the cleanup.

A later patch will document the design constraint implemented
here.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
[visit_start_alternate()'s assertion tightened, commit message tweaked]
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 42a502a7a60632234f0dd5028924926a7eac6c94
      
https://github.com/qemu/qemu/commit/42a502a7a60632234f0dd5028924926a7eac6c94
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M include/qapi/qmp/dispatch.h
    M qapi/qmp-dispatch.c
    M qapi/qmp-registry.c

  Log Message:
  -----------
  qmp: Drop dead command->type

Ever since QMP was first added back in commit 43c20a43, we have
never had any QmpCommandType other than QCT_NORMAL.  It's
pointless to carry around the cruft.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: b471d012e5d7bec1d2272738141e121b5581fcdf
      
https://github.com/qemu/qemu/commit/b471d012e5d7bec1d2272738141e121b5581fcdf
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qapi/qmp-input-visitor.c

  Log Message:
  -----------
  qmp-input: Clean up stack handling

Management of the top of stack was a bit verbose; creating a
temporary variable and adding some comments makes the existing
code more legible before the next few patches improve things.
No semantic changes other than asserting that we are always
visiting a QObject, and not a NULL value.  In particular, the
check for 'name && qobject_type(qobj) == QTYPE_QDICT)' is a
bit overkill (a dict visit should always have a name); a later
patch revisits that, while this patch is only changing one
layer of indentation due to dropping 'if (qobj)'.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: fc471c18d5d2ec713d5a019f9530398675494bc8
      
https://github.com/qemu/qemu/commit/fc471c18d5d2ec713d5a019f9530398675494bc8
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M docs/qapi-code-gen.txt
    M include/qapi/qmp-input-visitor.h
    M qapi/qmp-input-visitor.c
    M qmp.c
    M qom/qom-qobject.c
    M replay/replay-input.c
    M scripts/qapi-commands.py
    M tests/test-qmp-commands.c
    M tests/test-qmp-input-strict.c
    M tests/test-qmp-input-visitor.c
    M tests/test-visitor-serialization.c
    M util/qemu-sockets.c

  Log Message:
  -----------
  qapi: Consolidate QMP input visitor creation

Rather than having two separate ways to create a QMP input
visitor, where the safer approach has the more verbose name,
it is better to consolidate things into a single function
where the caller must explicitly choose whether to be strict
or to ignore excess input.  This patch is the strictly
mechanical conversion; the next patch will then audit which
uses can be made stricter.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 240f64b6dc3346d044d7beb7cc3a53668ce47384
      
https://github.com/qemu/qemu/commit/240f64b6dc3346d044d7beb7cc3a53668ce47384
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qmp.c
    M qom/qom-qobject.c
    M replay/replay-input.c
    M tests/test-qmp-commands.c
    M tests/test-visitor-serialization.c
    M util/qemu-sockets.c

  Log Message:
  -----------
  qapi: Use strict QMP input visitor in more places

The following uses of a QMP input visitor should be strict
(that is, excess keys in QDict input should be flagged if not
converted to QAPI):

- Testsuite code unrelated to explicitly testing non-strict
mode (test-qmp-commands, test-visitor-serialization); since
we want more code to be strict by default, having more tests
of strict mode doesn't hurt

- Code used for cloning QAPI objects (replay-input.c,
qemu-sockets.c); we are reparsing a QObject just barely
produced by the qmp output visitor and which therefore should
not have any garbage, so while it is extra work to be strict,
it validates that our clone is correct [note that a later patch
series will simplify these two uses by creating an actual
clone visitor that is much more efficient than a
generate/reparse cycle]

- qmp_object_add(), which calls into user_creatable_add_type().
Since command line parsing for '-object' uses the same
user_creatable_add_type() through the OptsVisitor, and that is
always strict, we want to ensure that any nested dictionaries
would be treated the same in QMP and from the command line (I
don't actually know if such nested dictionaries exist).  Note
that on this code change, strictness only matters for nested
dictionaries (if even possible), since we already flag excess
input at the top level during an earlier object_property_set()
on an unknown key, whether from QemuOpts:

$ ./x86_64-softmmu/qemu-system-x86_64 -nographic -nodefaults -qmp stdio -object 
secret,id=sec0,data=letmein,format=raw,foo=bar
qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: 
Property '.foo' not found

or from QMP:

$ ./x86_64-softmmu/qemu-system-x86_64 -nographic -nodefaults -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 93, "minor": 5, "major": 2}, "package": 
""}, "capabilities": []}}
{"execute":"qmp_capabilities"}
{"return": {}}
{"execute":"object-add","arguments":{"qom-type":"secret","id":"sec0","props":{"format":"raw","data":"letmein","foo":"bar"}}}
{"error": {"class": "GenericError", "desc": "Property '.foo' not found"}}

The only remaining uses of non-strict input visits are:

- QMP 'qom-set' (which eventually executes
object_property_set_qobject()) - mark it as something to revisit
in the future (I didn't want to spend any more time on this patch
auditing if we have any QOM dictionary properties that might be
impacted, and couldn't easily prove whether this code path is
shared with anything else).

- test-qmp-input-visitor: explicit tests of non-strict mode. If
we later get rid of users that don't need strictness, then this
test should be merged with test-qmp-input-strict

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: e5826a2fd727f0be54a81083f31fe02a275465cd
      
https://github.com/qemu/qemu/commit/e5826a2fd727f0be54a81083f31fe02a275465cd
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qapi/qmp-input-visitor.c

  Log Message:
  -----------
  qmp-input: Don't consume input when checking has_member

Commit e8316d7 mistakenly passed consume=true within
qmp_input_optional() when checking if an optional member was
present, but the mistake was silently ignored since the code
happily let us extract a member more than once.  Fix
qmp_input_optional() to not consume anything, then tighten up
the input visitor to ensure that a member is consumed exactly
once (all generated code follows this pattern; and the new
assert will catch any hand-written code that tries to visit
the same key more than once).

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: ed8415351941715f606749d6cdd64553b0de3f01
      
https://github.com/qemu/qemu/commit/ed8415351941715f606749d6cdd64553b0de3f01
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M docs/qapi-code-gen.txt
    M scripts/qapi-commands.py

  Log Message:
  -----------
  qapi-commands: Wrap argument visit in visit_start_struct

The qmp-input visitor was allowing callers to play rather fast
and loose: when visiting a QDict, you could grab members of the
root dictionary without first pushing into the dict; among the
culprit callers was the generated marshal code on the 'arguments'
dictionary of a QMP command.  But we are about to tighten the
input visitor, at which point the generated marshal code MUST
follow the same paradigms as everyone else, of pushing into the
struct before grabbing its keys.

Generated code grows as follows:

|@@ -515,7 +641,12 @@ void qmp_marshal_blockdev_backup(QDict *
|     BlockdevBackup arg = {0};
|
|     v = qmp_input_get_visitor(qiv);
|+    visit_start_struct(v, NULL, NULL, 0, &err);
|+    if (err) {
|+        goto out;
|+    }
|     visit_type_BlockdevBackup_members(v, &arg, &err);
|+    visit_end_struct(v, err ? NULL : &err);
|     if (err) {
|         goto out;
|     }
|@@ -527,7 +715,9 @@ out:
|     qmp_input_visitor_cleanup(qiv);
|     qdv = qapi_dealloc_visitor_new();
|     v = qapi_dealloc_get_visitor(qdv);
|+    visit_start_struct(v, NULL, NULL, 0, NULL);
|     visit_type_BlockdevBackup_members(v, &arg, NULL);
|+    visit_end_struct(v, NULL);
|     qapi_dealloc_visitor_cleanup(qdv);
| }

The use of 'err ? NULL : &err' is temporary; a later patch will
clean that up when it splits visit_end_struct().

Prior to this patch, the fact that there was no final
visit_end_struct() meant that even though we are using a strict
input visit, the marshalling code was not detecting excess input
at the top level (only in nested levels).  Fortunately, we have
code in monitor.c:qmp_check_client_args() that also checks for
no excess arguments at the top level.  But as the generated code
is more compact than the manual check, a later patch will clean
up monitor.c to drop the redundancy added here.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: ad739706bbadee49f164b4b7f4c7f5454ddf83cd
      
https://github.com/qemu/qemu/commit/ad739706bbadee49f164b4b7f4c7f5454ddf83cd
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qom/object_interfaces.c

  Log Message:
  -----------
  qom: Wrap prop visit in visit_start_struct

The qmp-input visitor was allowing callers to play rather fast
and loose: when visiting a QDict, you could grab members of the
root dictionary without first pushing into the dict; the final
such culprit was the QOM code for converting to and from object
properties.  But we are about to tighten the input visitor, at
which point user_creatable_add_type() as called with a QMP input
visitor via qmp_object_add() MUST follow the same paradigms as
everyone else, of pushing into the struct before grabbing its
keys.

The use of 'err ? NULL : &err' is temporary; a later patch will
clean that up when it splits visit_end_struct().

Furthermore, note that both callers always pass qdict, so we can
convert the conditional into an assert and reduce indentation.

The change has no impact to the testsuite now, but is required to
avoid a failure in tests/test-netfilter once qmp-input is made
stricter to detect inconsistent 'name' arguments on the root visit.

Since user_creatable_add_type() is also called with OptsVisitor
through user_creatable_add_opts(), we must also check that there
is no negative impact there; both pre- and post-patch, we see:

$ ./x86_64-softmmu/qemu-system-x86_64 -nographic -nodefaults -qmp stdio -object 
secret,id=sec0,data=letmein,format=raw,foo=bar
qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: 
Property '.foo' not found

That is, the only new checking that the new visit_end_struct() can
perform is for excess input, but we already catch excess input
earlier in object_property_set().

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: ce140b176920b5b65184020735a3c65ed3e9aeda
      
https://github.com/qemu/qemu/commit/ce140b176920b5b65184020735a3c65ed3e9aeda
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qapi/qmp-input-visitor.c

  Log Message:
  -----------
  qmp-input: Require struct push to visit members of top dict

Don't embed the root of the visit into the stack of current
containers being visited.  That way, we no longer get confused
on whether the first visit of a dictionary is to the dictionary
itself or to one of the members of the dictionary, based on
whether the caller passed name=NULL; and makes the QMP Input
visitor like other visitors where the value of 'name' is now
ignored on the root visit.  (We may someday want to revisit
the rules on what 'name' should be on a top-level visit,
rather than just ignoring it; but that would be the topic of
another patch).

An audit of all qmp_input_visitor_new() call sites shows that
there were only two places where callers had previously been
visiting to a QDict with a non-NULL name to bypass a call to
visit_start_struct(), and those were fixed in prior patches.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: fcf3cb21783b2dae3358fdbe7001cb2f74e0cedf
      
https://github.com/qemu/qemu/commit/fcf3cb21783b2dae3358fdbe7001cb2f74e0cedf
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qapi/qmp-input-visitor.c

  Log Message:
  -----------
  qmp-input: Refactor when list is advanced

In the QMP input visitor, visiting a list traverses two objects:
the QAPI GenericList of the caller (which gets advanced in
visit_next_list() regardless of this patch), and the QList input
that we are converting to QAPI.  For consistency with QDict
visits, we want to consume elements from the input QList during
the visit_type_FOO() for the list element; that is, we want ALL
the code for consuming an input to live in qmp_input_get_object(),
rather than having it split according to whether we are visiting
a dict or a list.  Making qmp_input_get_object() the common point
of consumption will make it easier for a later patch to refactor
visit_start_list() to cover the GenericList * head of a QAPI list,
and in turn will get rid of the 'first' flag (which lived in
qmp_input_next_list() pre-patch, and is hoisted to StackObject
by this patch).

This patch is therefore altering the post-condition use of 'entry',
while keeping what gets visited unchanged, from:
   start_list next_list type_ELT ... next_list type_ELT next_list end_list
 visits                      1st elt                last elt
 entry  NULL       1st elt   1st elt      last elt  last elt NULL      gone

where type_ELT() returns (entry ? entry : 1st elt) and next_list() steps
entry

to this usage:
   start_list next_list type_ELT ... next_list type_ELT next_list end_list
 visits                      1st elt                last elt
 entry  1st elt    1nd elt   2nd elt      last elt  NULL     NULL      gone

where type_ELT() steps entry and returns the old entry, and next_list()
leaves entry alone.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: adfb264c9ed04bfc694921b72173be8e29e90024
      
https://github.com/qemu/qemu/commit/adfb264c9ed04bfc694921b72173be8e29e90024
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M include/qapi/dealloc-visitor.h
    M include/qapi/opts-visitor.h
    M include/qapi/string-input-visitor.h
    M include/qapi/string-output-visitor.h
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/qapi-visit-core.c

  Log Message:
  -----------
  qapi: Document visitor interfaces, add assertions

The visitor interface for mapping between QObject/QemuOpts/string
and QAPI is scandalously under-documented, making changes to visitor
core, individual visitors, and users of visitors difficult to
coordinate.  Among other questions: when is it safe to pass NULL,
vs. when a string must be provided; which visitors implement which
callbacks; the difference between concrete and virtual visits.

Correct this by retrofitting proper contracts, and document where some
of the interface warts remain (for example, we may want to modify
visit_end_* to require the same 'obj' as the visit_start counterpart,
so the dealloc visitor can be simplified).  Later patches in this
series will tackle some, but not all, of these warts.

Add assertions to (partially) enforce the contract.  Some of these
were only made possible by recent cleanup commits.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
[Doc fix from Eric squashed in]
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 7d7a337ec32c77f11ba04da412752b35e18d3c5c
      
https://github.com/qemu/qemu/commit/7d7a337ec32c77f11ba04da412752b35e18d3c5c
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M tests/.gitignore
    M tests/Makefile
    A tests/check-qnull.c
    M tests/test-qmp-output-visitor.c

  Log Message:
  -----------
  tests: Add check-qnull

Add a new test, for checking reference counting of qnull(). As
part of the new file, move a previous reference counting change
added in commit a861564 to a more logical place.

Note that while most of the check-q*.c leave visitor stuff to
the test-qmp-*-visitor.c, in this case we actually want the
visitor tests in our new file because we are validating the
reference count of qnull_, which is an internal detail that
test-qmp-*-visitor should not be peeking into (or put another
way, qnull() is the only special case where we don't have
independent allocation of a QObject, so none of the other
visitor tests require the layering violation present in this
test).

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 3bc97fd5924561d92f32758c67eaffd2e4e25038
      
https://github.com/qemu/qemu/commit/3bc97fd5924561d92f32758c67eaffd2e4e25038
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M include/qapi/opts-visitor.h
    M include/qapi/string-input-visitor.h
    M include/qapi/string-output-visitor.h
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qmp-input-visitor.c
    M qapi/qmp-output-visitor.c

  Log Message:
  -----------
  qapi: Add visit_type_null() visitor

Right now, qmp-output-visitor happens to produce a QNull result
if nothing is actually visited between the creation of the visitor
and the request for the resulting QObject.  A stronger protocol
would require that a QMP output visit MUST visit something.  But
to still be able to produce a JSON 'null' output, we need a new
visitor function that states our intentions.  Yes, we could say
that such a visit must go through visit_type_any(), but that
feels clunky.

So this patch introduces the new visit_type_null() interface and
its no-op interface in the dealloc visitor, and stubs in the
qmp visitors (the next patch will finish the implementation).
For the visitors that will not implement the callback, document
the situation. The code in qapi-visit-core unconditionally
dereferences the callback pointer, so that a segfault will inform
a developer if they need to implement the callback for their
choice of visitor.

Note that JSON has a primitive null type, with the single value
null; likewise with the QNull type for QObject; but for QAPI,
we just have the 'null' value without a null type.  We may
eventually want to add more support in QAPI for null (most likely,
we'd use it via an alternate type that permits 'null' or an
object); but we'll create that usage when we need it.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 3df016f185521f8dfa5bd89168722887156405c7
      
https://github.com/qemu/qemu/commit/3df016f185521f8dfa5bd89168722887156405c7
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qapi/qmp-input-visitor.c
    M qapi/qmp-output-visitor.c
    M tests/check-qnull.c
    M tests/test-qmp-input-visitor.c
    M tests/test-qmp-output-visitor.c

  Log Message:
  -----------
  qmp: Support explicit null during visits

Implement the new type_null() callback for the qmp input and
output visitors. While we don't yet have a use for this in QAPI
input (the generator will need some tweaks first), some
potential usages have already been discussed on the list.
Meanwhile, the output visitor could already output explicit null
via type_any, but this gives us finer control.

At any rate, it's easy to test that we can round-trip an explicit
null through manual use of visit_type_null() wrapped by a virtual
visit_start_struct() walk, even if we can't do the visit in a
QAPI type.  Repurpose the test_visitor_out_empty test,
particularly since a future patch will tighten semantics to
forbid use of qmp_output_get_qobject() without at least one
intervening visit_type_*.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: a543a554cfffa4bbed2c74ac56c1abf046821377
      
https://github.com/qemu/qemu/commit/a543a554cfffa4bbed2c74ac56c1abf046821377
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M hw/ppc/spapr_drc.c

  Log Message:
  -----------
  spapr_drc: Expose 'null' in qom-get when there is no fdt

Now that the QMP output visitor supports an explicit null
output, we should utilize it to make it easier to diagnose
the difference between a missing fdt ('null') vs. a
present-but-empty one ('{}').

(Note that this reverts the behavior of commit ab8bf1d, taking
us back to the behavior of commit 6c2f9a1 [which in turn
stemmed from a crash fix in 1d10b44]; but that this time,
the change is intentional and not an accidental side-effect.)

Signed-off-by: Eric Blake <address@hidden>
Acked-by: David Gibson <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: f2ff429bfa713da1366333417fecfa939a709cf1
      
https://github.com/qemu/qemu/commit/f2ff429bfa713da1366333417fecfa939a709cf1
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M tests/test-qmp-output-visitor.c

  Log Message:
  -----------
  qmp: Don't reuse qmp visitor after grabbing output

The testsuite was the only client that attempted to reuse a
QmpOutputVisitor for a second visit after encountering an
error and/or calling qmp_output_get_qobject() on a first
visit.  The next patch is about to tighten the semantics to
be one-shot usage of the visitor, like all other visitors
(which will enable further simplifications down the road).

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 56a6f02b8ce1fe41a2a9077593e46eca7d98267d
      
https://github.com/qemu/qemu/commit/56a6f02b8ce1fe41a2a9077593e46eca7d98267d
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qapi/qmp-output-visitor.c

  Log Message:
  -----------
  qmp: Tighten output visitor rules

Tighten assertions in the QMP output visitor, so that:

- qmp_output_get_qobject() can only be called after pairing a
visit_end_* for every visit_start_* (rather than allowing it on
a partially built object)

- qmp_output_get_qobject() cannot be called unless at least one
visit_type_* or visit_start/visit_end pair has occurred since
creation/reset (the accidental return of NULL fixed by commit
ab8bf1d7 would have been much easier to diagnose)

- ensure that we are encountering the expected object or list
type, to provide protection against mismatched push(struct)/
pop(list) or push(list)/pop(struct), similar to the qmp-input
protection added in commit bdd8e6b5.

- ensure that except for the root, 'name' is non-null inside a
dict, and NULL inside a list (this may need changing later if
we add "name.0" support for better error messages for a list,
but for now it makes sure all users are at least consistent)

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 15c2f669e3fb2bc97f7b42d1871f595c0ac24af8
      
https://github.com/qemu/qemu/commit/15c2f669e3fb2bc97f7b42d1871f595c0ac24af8
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M block/crypto.c
    M docs/qapi-code-gen.txt
    M hw/ppc/spapr_drc.c
    M hw/virtio/virtio-balloon.c
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/opts-visitor.c
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qmp-input-visitor.c
    M qapi/qmp-output-visitor.c
    M qom/object.c
    M qom/object_interfaces.c
    M scripts/qapi-commands.py
    M scripts/qapi-event.py
    M scripts/qapi-visit.py
    M tests/test-qmp-input-visitor.c
    M tests/test-qmp-output-visitor.c

  Log Message:
  -----------
  qapi: Split visit_end_struct() into pieces

As mentioned in previous patches, we want to call visit_end_struct()
functions unconditionally, so that visitors can release resources
tied up since the matching visit_start_struct() without also having
to worry about error priority if more than one error occurs.

Even though error_propagate() can be safely used to ignore a second
error during cleanup caused by a first error, it is simpler if the
cleanup cannot set an error.  So, split out the error checking
portion (basically, input visitors checking for unvisited keys) into
a new function visit_check_struct(), which can be safely skipped if
any earlier errors are encountered, and leave the cleanup portion
(which never fails, but must be called unconditionally if
visit_start_struct() succeeded) in visit_end_struct().

Generated code in qapi-visit.c has diffs resembling:

|@@ -59,10 +59,12 @@ void visit_type_ACPIOSTInfo(Visitor *v,
|         goto out_obj;
|     }
|     visit_type_ACPIOSTInfo_members(v, obj, &err);
|-    error_propagate(errp, err);
|-    err = NULL;
|+    if (err) {
|+        goto out_obj;
|+    }
|+    visit_check_struct(v, &err);
| out_obj:
|-    visit_end_struct(v, &err);
|+    visit_end_struct(v);
| out:

and in qapi-event.c:

@@ -47,7 +47,10 @@ void qapi_event_send_acpi_device_ost(ACP
|         goto out;
|     }
|     visit_type_q_obj_ACPI_DEVICE_OST_arg_members(v, &param, &err);
|-    visit_end_struct(v, err ? NULL : &err);
|+    if (!err) {
|+        visit_check_struct(v, &err);
|+    }
|+    visit_end_struct(v);
|     if (err) {
|         goto out;

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
[Conflict with a doc fixup resolved]
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 7337468385f0ad258657b2ce76ac0a7306f9f186
      
https://github.com/qemu/qemu/commit/7337468385f0ad258657b2ce76ac0a7306f9f186
  Author: Markus Armbruster <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M tests/test-string-input-visitor.c

  Log Message:
  -----------
  tests/string-input-visitor: Add negative integer tests

Add two negative tests, one for int and one for int16List.  The latter
exposes a bug: nonsensical input results in an empty list instead of
an error.

Signed-off-by: Markus Armbruster <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>


  Commit: 74f24cb6306d065045d0e2215a7d10533fa59c57
      
https://github.com/qemu/qemu/commit/74f24cb6306d065045d0e2215a7d10533fa59c57
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M qapi/string-input-visitor.c
    M tests/test-string-input-visitor.c

  Log Message:
  -----------
  qapi: Fix string input visitor handling of invalid list

As shown in the previous commit, the string input visitor was
treating bogus input as an empty list rather than an error.
Fix parse_str() to set errp, then the callers to exit early if
an error was reported.

Meanwhile, fix the testsuite to use the generated
qapi_free_int16List() instead of rolling our own, and to
validate the fixed behavior, while at the same time documenting
one more change that we'd like to make in a later patch (a
failed visit_start_list should guarantee a NULL pointer,
regardless of what things were on input).

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: d9f62dde1303286b24ac8ce88be27e2b9b9c5f46
      
https://github.com/qemu/qemu/commit/d9f62dde1303286b24ac8ce88be27e2b9b9c5f46
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M docs/qapi-code-gen.txt
    M hw/ppc/spapr_drc.c
    M include/qapi/opts-visitor.h
    M include/qapi/string-input-visitor.h
    M include/qapi/string-output-visitor.h
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/opts-visitor.c
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qmp-input-visitor.c
    M qapi/qmp-output-visitor.c
    M qapi/string-input-visitor.c
    M qapi/string-output-visitor.c
    M scripts/qapi-visit.py
    M tests/test-string-input-visitor.c

  Log Message:
  -----------
  qapi: Simplify semantics of visit_next_list()

The semantics of the list visit are somewhat baroque, with the
following pseudocode when FooList is used:

start()
for (prev = head; cur = next(prev); prev = &cur) {
    visit(&cur->value)
}

Note that these semantics (advance before visit) requires that
the first call to next() return the list head, while all other
calls return the next element of the list; that is, every visitor
implementation is required to track extra state to decide whether
to return the input as-is, or to advance.  It also requires an
argument of 'GenericList **' to next(), solely because the first
iteration might need to modify the caller's GenericList head, so
that all other calls have to do a layer of dereferencing.

Thankfully, we only have two uses of list visits in the entire
code base: one in spapr_drc (which completely avoids
visit_next_list(), feeding in integers from a different source
than uint8List), and one in qapi-visit.py.  That is, all other
list visitors are generated in qapi-visit.c, and share the same
paradigm based on a qapi FooList type, so we can refactor how
lists are laid out with minimal churn among clients.

We can greatly simplify things by hoisting the special case
into the start() routine, and flipping the order in the loop
to visit before advance:

start(head)
for (tail = *head; tail; tail = next(tail)) {
    visit(&tail->value)
}

With the simpler semantics, visitors have less state to track,
the argument to next() is reduced to 'GenericList *', and it
also becomes obvious whether an input visitor is allocating a
FooList during visit_start_list() (rather than the old way of
not knowing if an allocation happened until the first
visit_next_list()).  As a minor drawback, we now allocate in
two functions instead of one, and have to pass the size to
both functions (unless we were to tweak the input visitors to
cache the size to start_list for reuse during next_list, but
that defeats the goal of less visitor state).

The signature of visit_start_list() is chosen to match
visit_start_struct(), with the new parameters after 'name'.

The spapr_drc case is a virtual visit, done by passing NULL for
list, similarly to how NULL is passed to visit_start_struct()
when a qapi type is not used in those visits.  It was easy to
provide these semantics for qmp-output and dealloc visitors,
and a bit harder for qmp-input (several prerequisite patches
refactored things to make this patch straightforward).  But it
turned out that the string and opts visitors munge enough other
state during visit_next_list() to make it easier to just
document and require a GenericList visit for now; an assertion
will remind us to adjust things if we need the semantics in the
future.

Several pre-requisite cleanup patches made the reshuffling of
the various visitors easier; particularly the qmp input visitor.

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 68ab47e4b4ecc1c4649362b8cc1e49794d1a6537
      
https://github.com/qemu/qemu/commit/68ab47e4b4ecc1c4649362b8cc1e49794d1a6537
  Author: Eric Blake <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M docs/qapi-code-gen.txt
    M include/qapi/visitor.h
    M qapi/qapi-visit-core.c
    M scripts/qapi-visit.py
    M tests/test-qmp-commands.c
    M tests/test-qmp-input-strict.c
    M tests/test-qmp-input-visitor.c

  Log Message:
  -----------
  qapi: Change visit_type_FOO() to no longer return partial objects

Returning a partial object on error is an invitation for a careless
caller to leak memory.  We already fixed things in an earlier
patch to guarantee NULL if visit_start fails ("qapi: Guarantee
NULL obj on input visitor callback error"), but that does not
help the case where visit_start succeeds but some other failure
happens before visit_end, such that we leak a partially constructed
object outside visit_type_FOO(). As no one outside the testsuite
was actually relying on these semantics, it is cleaner to just
document and guarantee that ALL pointer-based visit_type_FOO()
functions always leave a safe value in *obj during an input visitor
(either the new object on success, or NULL if an error is
encountered), so callers can now unconditionally use
qapi_free_FOO() to clean up regardless of whether an error occurred.

The decision is done by adding visit_is_input(), then updating the
generated code to check if additional cleanup is needed based on
the type of visitor in use.

Note that we still leave *obj unchanged after a scalar-based
visit_type_FOO(); I did not feel like auditing all uses of
visit_type_Enum() to see if the callers would tolerate a specific
sentinel value (not to mention having to decide whether it would
be better to use 0 or ENUM__MAX as that sentinel).

Signed-off-by: Eric Blake <address@hidden>
Message-Id: <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 6ddeeffffecf1f78acf6c93cbf267a8abe755836
      
https://github.com/qemu/qemu/commit/6ddeeffffecf1f78acf6c93cbf267a8abe755836
  Author: Peter Maydell <address@hidden>
  Date:   2016-05-12 (Thu, 12 May 2016)

  Changed paths:
    M block/crypto.c
    M docs/qapi-code-gen.txt
    M hw/ppc/spapr_drc.c
    M hw/virtio/virtio-balloon.c
    M include/qapi/dealloc-visitor.h
    M include/qapi/opts-visitor.h
    M include/qapi/qmp-input-visitor.h
    M include/qapi/qmp/dispatch.h
    M include/qapi/string-input-visitor.h
    M include/qapi/string-output-visitor.h
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/opts-visitor.c
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qmp-dispatch.c
    M qapi/qmp-input-visitor.c
    M qapi/qmp-output-visitor.c
    M qapi/qmp-registry.c
    M qapi/string-input-visitor.c
    M qapi/string-output-visitor.c
    M qmp.c
    M qom/object.c
    M qom/object_interfaces.c
    M qom/qom-qobject.c
    M replay/replay-input.c
    M scripts/qapi-commands.py
    M scripts/qapi-event.py
    M scripts/qapi-visit.py
    M tests/.gitignore
    M tests/Makefile
    A tests/check-qnull.c
    M tests/test-qmp-commands.c
    M tests/test-qmp-input-strict.c
    M tests/test-qmp-input-visitor.c
    M tests/test-qmp-output-visitor.c
    M tests/test-string-input-visitor.c
    M tests/test-visitor-serialization.c
    M util/qemu-sockets.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2016-05-12' into 
staging

QAPI patches for 2016-05-12

# gpg: Signature made Thu 12 May 2016 08:49:04 BST using RSA key ID EB918653
# gpg: Good signature from "Markus Armbruster <address@hidden>"
# gpg:                 aka "Markus Armbruster <address@hidden>"

* remotes/armbru/tags/pull-qapi-2016-05-12: (23 commits)
  qapi: Change visit_type_FOO() to no longer return partial objects
  qapi: Simplify semantics of visit_next_list()
  qapi: Fix string input visitor handling of invalid list
  tests/string-input-visitor: Add negative integer tests
  qapi: Split visit_end_struct() into pieces
  qmp: Tighten output visitor rules
  qmp: Don't reuse qmp visitor after grabbing output
  spapr_drc: Expose 'null' in qom-get when there is no fdt
  qmp: Support explicit null during visits
  qapi: Add visit_type_null() visitor
  tests: Add check-qnull
  qapi: Document visitor interfaces, add assertions
  qmp-input: Refactor when list is advanced
  qmp-input: Require struct push to visit members of top dict
  qom: Wrap prop visit in visit_start_struct
  qapi-commands: Wrap argument visit in visit_start_struct
  qmp-input: Don't consume input when checking has_member
  qapi: Use strict QMP input visitor in more places
  qapi: Consolidate QMP input visitor creation
  qmp-input: Clean up stack handling
  ...

Signed-off-by: Peter Maydell <address@hidden>


Compare: https://github.com/qemu/qemu/compare/f83b70f70192...6ddeeffffecf