[Qemu-commits] [qemu/qemu] 006ca0: qapi: Separate type QNull from QObject

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/qemu/qemu
  Commit: 006ca09f3027d86346fce707e9295975c6558f42
      
https://github.com/qemu/qemu/commit/006ca09f3027d86346fce707e9295975c6558f42
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M include/qapi/qmp/qobject.h
    M qapi/qobject-output-visitor.c
    M qobject/json-parser.c
    M qobject/qnull.c
    M target/i386/cpu.c
    M tests/check-qjson.c
    M tests/check-qnull.c

  Log Message:
  -----------
  qapi: Separate type QNull from QObject

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>


  Commit: d2f95f4d482374485234790a6fc3cca29ebb7355
      
https://github.com/qemu/qemu/commit/d2f95f4d482374485234790a6fc3cca29ebb7355
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M hw/ppc/spapr_drc.c
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M qapi/qapi-clone-visitor.c
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qobject-input-visitor.c
    M qapi/qobject-output-visitor.c
    M qapi/string-input-visitor.c
    M qapi/string-output-visitor.c
    M qapi/trace-events
    M target/ppc/translate_init.c
    M tests/check-qnull.c
    M tests/test-qobject-input-visitor.c
    M tests/test-qobject-output-visitor.c

  Log Message:
  -----------
  qapi: Use QNull for a more regular visit_type_null()

Make visit_type_null() take an @obj argument like its buddies.  This
helps keep the next commit simple.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>


  Commit: 4d2d5c41a9e8ee201cda8be8701f7f9fc92e71aa
      
https://github.com/qemu/qemu/commit/4d2d5c41a9e8ee201cda8be8701f7f9fc92e71aa
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M docs/devel/qapi-code-gen.txt
    M include/qapi/qmp/qobject.h
    M include/qemu/typedefs.h
    M scripts/qapi.py
    M tests/qapi-schema/qapi-schema-test.json
    M tests/qapi-schema/qapi-schema-test.out
    M tests/test-qobject-input-visitor.c
    M tests/test-qobject-output-visitor.c

  Log Message:
  -----------
  qapi: Introduce a first class 'null' type

I expect the 'null' type to be useful mostly for members of alternate
types.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Signed-off-by: Markus Armbruster <address@hidden>


  Commit: 06f80154b2ada1d58ac504e98ff6e943b069b96c
      
https://github.com/qemu/qemu/commit/06f80154b2ada1d58ac504e98ff6e943b069b96c
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

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

  Log Message:
  -----------
  tests/test-qobject-input-visitor: Drop redundant test

test_visitor_in_alternate() tests UserDefAlternate with inadmissible
input.  test_visitor_in_fail_alternate() does basically the same.
Drop the former, keep the latter.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>


  Commit: c42e8742f527476839bcc5f91c3d2ea456ca6a45
      
https://github.com/qemu/qemu/commit/c42e8742f527476839bcc5f91c3d2ea456ca6a45
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M blockdev.c
    M qapi/block-core.json
    M tests/qemu-iotests/085
    M tests/qemu-iotests/139

  Log Message:
  -----------
  block: Use JSON null instead of "" to disable backing file

BlockdevRef is an alternate of BlockdevOptions (inline definition) and
str (reference to an existing block device by name).  BlockdevRef
value "" is special: "no block device should be referenced."  It's
actually interpreted that way in just one place: optional member
@backing of COW formats.  Semantics:

* Present means "use this block device" as backing storage

* Absent means "default to the one stored in the image"

* Except "" means "don't use backing storage at all"

The first two are perfectly normal: when the parameter is absent, it
defaults to an implied value, but the value's meaning is the same.

The third one overloads the parameter with a second meaning.  The
overloading is *implicit*, i.e. it's not visible in the types.  Works
here, because "" is not a value block device ID.

Pressing argument values the schema accepts, but are semantically
invalid, into service to mean "do something else entirely" is not
general, as suitable invalid values need not exist.  I also find it
ugly.

To clean this up, we could add a separate flag argument to suppress
@backing, or add a distinct value to @backing.  This commit implements
the latter: add JSON null to the values of @backing, deprecate "".

Because we're so close to the 2.10 freeze, implement it in the
stupidest way possible: have qmp_blockdev_add() rewrite null to ""
before anything else can see the null.  Works, because BlockdevRef
occurs only within arguments of blockdev-add.  The proper way to do it
would be rewriting "" to null, preferably in a cleaner way, but that
requires fixing up code to work with null.  Add a TODO comment for
that.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
Acked-by: Kevin Wolf <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 7e91e82044f8d23acfb9949a2cdd667a6b239acd
      
https://github.com/qemu/qemu/commit/7e91e82044f8d23acfb9949a2cdd667a6b239acd
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M hmp.c

  Log Message:
  -----------
  hmp: Clean up and simplify hmp_migrate_set_parameter()

The bulk of hmp_migrate_set_parameter()'s code sets one member of
MigrationParameters according to the command's arguments.  It uses a
string visitor for integer and boolean members, but not for string and
size members.  It calls visit_type_bool() right away, but delays
visit_type_int() some.  The delaying requires a flag variable and a
bit of trickery: we set all integer members instead of just the one we
want, and rely on the has_FOOs to mask the unwanted ones.

Clean this up as follows.  Don't delay calling visit_type_int().  Use
the string visitor for strings, too.  This involves extra allocations
and cleanup, but doing them is simpler and cleaner than avoiding them.

Sadly, using the string visitor for sizes isn't possible, because it
defaults to Bytes rather than Mebibytes.  Add a comment explaining
that.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 8cc99dcdc264bc896926e43e7576c7b7ab633d70
      
https://github.com/qemu/qemu/commit/8cc99dcdc264bc896926e43e7576c7b7ab633d70
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M hmp.c
    M migration/migration.c
    M qapi-schema.json

  Log Message:
  -----------
  migration: Clean up around tls_creds, tls_hostname

Optional MigrationParameters members tls_creds and tls_hostname can't
actually be absent outside qmp_migrate_set_parameters() since commit
4af245d (v2.9.0).

Note that commit 4af245d reverted the part of commit de63ab6 (v2.8.0)
that made tls_creds and tls_hostname absent instead of "" in the value
of query-migrate-parameters, even though commit de63ab6 called that a
mistake.  What a mess.

Drop the redundant tests for presence, and update documentation.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Reviewed-by: Dr. David Alan Gilbert <address@hidden>


  Commit: e87fae4c488ff8a10b921311a63f16b94031c611
      
https://github.com/qemu/qemu/commit/e87fae4c488ff8a10b921311a63f16b94031c611
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M migration/migration.c

  Log Message:
  -----------
  migration: Add TODO comments on duplication of QAPI_CLONE()

qmp_query_migrate_parameters() and qmp_migrate_set_parameters()
effectively duplicate QAPI_CLONE() inline.  Add suitable TODO
comments.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 1bda8b3c6950f74482ba19e8529db72b511ba977
      
https://github.com/qemu/qemu/commit/1bda8b3c6950f74482ba19e8529db72b511ba977
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M hmp.c
    M migration/migration.c
    M qapi-schema.json

  Log Message:
  -----------
  migration: Unshare MigrationParameters struct for now

Commit de63ab6 "migrate: Share common MigrationParameters struct"
reused MigrationParameters for the arguments of
migrate-set-parameters, with the following rationale:

    It is rather verbose, and slightly error-prone, to repeat
    the same set of parameters for input (migrate-set-parameters)
    as for output (query-migrate-parameters), where the only
    difference is whether the members are optional.  We can just
    document that the optional members will always be present
    on output, and then share a common struct between both
    commands.  The next patch can then reduce the amount of
    code needed on input.

I need to unshare them to correct a design flaw in a stupid, but
minimally invasive way, in the next commit.  We can restore the
sharing when we redo that patch in a less stupid way.  Add a suitable
TODO comment.

Note that I revert only the sharing part of commit de63ab6, not the
part that made the members of query-migrate-parameters' result
optional.  The schema (and thus introspection) remains inaccurate for
query-migrate-parameters.  If we decide not to restore the sharing, we
should revert that part, too.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: 01fa55982692fb51a16049b63b571651a1053989
      
https://github.com/qemu/qemu/commit/01fa55982692fb51a16049b63b571651a1053989
  Author: Markus Armbruster <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M hmp.c
    M migration/migration.c
    M qapi-schema.json

  Log Message:
  -----------
  migration: Use JSON null instead of "" to reset parameter to default

migrate-set-parameters sets migration parameters according to is
arguments like this:

* Present means "set the parameter to this value"

* Absent means "leave the parameter unchanged"

* Except for parameters tls_creds and tls_hostname, "" means "reset
  the parameter to its default value

The first two are perfectly normal: presence of the parameter makes
the command do something.

The third one overloads the parameter with a second meaning.  The
overloading is *implicit*, i.e. it's not visible in the types.  Works
here, because "" is neither a valid TLS credentials ID, nor a valid
host name.

Pressing argument values the schema accepts, but are semantically
invalid, into service to mean "reset to default" is not general, as
suitable invalid values need not exist.  I also find it ugly.

To clean this up, we could add a separate flag argument to ask for
"reset to default", or add a distinct value to @tls_creds and
@tls_hostname.  This commit implements the latter: add JSON null to
the values of @tls_creds and @tls_hostname, deprecate "".

Because we're so close to the 2.10 freeze, implement it in the
stupidest way possible: have qmp_migrate_set_parameters() rewrite null
to "" before anything else can see the null.  The proper way to do it
would be rewriting "" to null, but that requires fixing up code to
work with null.  Add TODO comments for that.

Signed-off-by: Markus Armbruster <address@hidden>
Reviewed-by: Daniel P. Berrange <address@hidden>
Reviewed-by: Eric Blake <address@hidden>


  Commit: cf44d31d35df3de9d4f49339254050195f777f3e
      
https://github.com/qemu/qemu/commit/cf44d31d35df3de9d4f49339254050195f777f3e
  Author: Peter Maydell <address@hidden>
  Date:   2017-07-24 (Mon, 24 Jul 2017)

  Changed paths:
    M blockdev.c
    M docs/devel/qapi-code-gen.txt
    M hmp.c
    M hw/ppc/spapr_drc.c
    M include/qapi/qmp/qobject.h
    M include/qapi/visitor-impl.h
    M include/qapi/visitor.h
    M include/qemu/typedefs.h
    M migration/migration.c
    M qapi-schema.json
    M qapi/block-core.json
    M qapi/qapi-clone-visitor.c
    M qapi/qapi-dealloc-visitor.c
    M qapi/qapi-visit-core.c
    M qapi/qobject-input-visitor.c
    M qapi/qobject-output-visitor.c
    M qapi/string-input-visitor.c
    M qapi/string-output-visitor.c
    M qapi/trace-events
    M qobject/json-parser.c
    M qobject/qnull.c
    M scripts/qapi.py
    M target/i386/cpu.c
    M target/ppc/translate_init.c
    M tests/check-qjson.c
    M tests/check-qnull.c
    M tests/qapi-schema/qapi-schema-test.json
    M tests/qapi-schema/qapi-schema-test.out
    M tests/qemu-iotests/085
    M tests/qemu-iotests/139
    M tests/test-qobject-input-visitor.c
    M tests/test-qobject-output-visitor.c

  Log Message:
  -----------
  Merge remote-tracking branch 'remotes/armbru/tags/pull-qapi-2017-07-18-v2' 
into staging

QAPI patches for 2017-07-18

# gpg: Signature made Mon 24 Jul 2017 12:40:56 BST
# gpg:                using RSA key 0x3870B400EB918653
# gpg: Good signature from "Markus Armbruster <address@hidden>"
# gpg:                 aka "Markus Armbruster <address@hidden>"
# Primary key fingerprint: 354B C8B3 D7EB 2A6B 6867  4E5F 3870 B400 EB91 8653

* remotes/armbru/tags/pull-qapi-2017-07-18-v2:
  migration: Use JSON null instead of "" to reset parameter to default
  migration: Unshare MigrationParameters struct for now
  migration: Add TODO comments on duplication of QAPI_CLONE()
  migration: Clean up around tls_creds, tls_hostname
  hmp: Clean up and simplify hmp_migrate_set_parameter()
  block: Use JSON null instead of "" to disable backing file
  tests/test-qobject-input-visitor: Drop redundant test
  qapi: Introduce a first class 'null' type
  qapi: Use QNull for a more regular visit_type_null()
  qapi: Separate type QNull from QObject

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


Compare: https://github.com/qemu/qemu/compare/5dd8990841a9...cf44d31d35df