[Qemu-devel] [PATCH v4 00/14] block: Try to create well-typed json:{} filenames

[Max Reitz]

Hi,

There are two explanations of this cover letter, a relative one (to v3)
and an absolute one.


*** Important note ***

The final patch in this series is an example that converts most of
block-core.json to use default values where possible.  We may decide to
take it or not.  It isn’t important for the main purpose of this series,
so I’d be very much fine with chopping it off.

(It does have a nice diff stat, though.)

*** Important note end ***


Relative explanation:

The actual functional goal of this series is to allow all blockdev
options that can be represented with -drive to have an equivalent with
-blockdev (safe for rbd’s =keyvalue-pairs).

To this end, qcow(2)’s encryption needs an “auto” format which can
automatically deduce the format from the image header.  To make things
nicer, I decided (already in v1) to make this format optional so users
could just specify encrypt.secret and let the format driver figure out
the rest.

Until v3, this was implemented by letting the discriminator of flat
unions be optional, as long as a default-value is provided.  Markus
(rightfully) complained that this is very specific and would be covered
by just having default values for QAPI struct members in general.
So now this version implements this.  This is a bit more complicated
than just implementing a default-variant, mainly because the latter only
needs to accept enum values, whereas a generally usable “default” should
accept values of all QAPI types (to the extent what is reasonable).

So what was (until v3)

  { 'union': 'Foo',
    'base': { '*discr': 'SomeEnum' },
    'discriminator': 'discr',
    'default-variant': 'value1',
    'data': { 'value1': 'Bar', 'value2': 'Baz' } }

becomes

  { 'union': 'Foo',
    'base': { '*discr': { 'type': 'SomeEnum', 'default': 'value1' } },
    'discriminator': 'discr',
    'data': { 'value1': 'Bar', 'value2': 'Baz' } }



Absolute explanation:

When qemu reports json:{} filename, it just uses whatever type you gave
an option in.  With -drive, all options are strings and they do not have
to pass the test of the typing firewall of the QAPI schema, so you just
get strings thrown back at you even if that does not match the schema.
(Also, if you use json:{} yourself, you’re free to give the options as
strings as well.)

Example:

$ ./qemu-img info --image-opts driver=raw,size=512,file.driver=null-co
image: json:{"driver": "raw", "size": "512", "file": {"driver": "null-co"}}

@size is supposed to be an integer, according to the schema, so the
correct result would be (which is what you get after this series):

$ ./qemu-img info --image-opts driver=raw,size=512,file.driver=null-co
image: json:{"driver": "raw", "size": 512, "file": {"driver": "null-co"}}


This is achieved by patch 11, which makes bdrv_refresh_filename() run
the options through the flat-confused input visitor, and then through
the output visitor to get all to the correct type.  If anything fails,
the result is as before (hence the “Try” in the title).

There are cases where this cannot work.  Those are the ones where -drive
accepts something that is not allowed by the QAPI schema.  One of these
cases is rbd’s =keyvalue-pairs, which is just broken altogether, so
let’s simply ignore that.  (I don’t think anybody’s going to complain
that the json:{} filename they get is not correctly typed after they’ve
used that option.)

The other case (I know of) is qcow(2)’s encryption.  In the QAPI schema,
encrypt.format is not optional because it is the discriminator for which
kind of options to use.  However, for -drive, it is optional because the
qcow2 driver can infer the encryption format from the image header.

The solution that’s proposed by this series is to make flat union
discriminators optional and provide a default.  This is accomplished by
generally allowing default values to be provided for QAPI struct
members.

Both AES and LUKS encryption allow only a key-secret option, so we can
add a new pseudo-format “auto” that accepts exactly that option and
makes the qcow2 driver read the real format from the image header.  This
pseudo-format is made the default for encrypt.format, and thus you can
then specify encrypt.key-secret without having to specify
encrypt.format (while still adhering to the QAPI schema).


So, in this series:
- The QAPI code generator is modified to allow default values for
  optional struct members.  This in turn allows flat union
  discriminators be optional, too, but only if a default value is
  provided.
  - Accordingly, documentation, tests, and introspection are adjusted.

- This is used to make qcow’s and qcow2’s encrypt.format parameter
  optional.  It now defaults to “from-image” which is a new
  pseudo-format that allows a key-secret to be given, and otherwise
  leaves it to the format driver to determine the encryption format.

- json:{} filenames are attempted to be typed correctly when they are
  generated, by running bs->full_open_options through a healthy mix of
  qdict_flatten(), the flat-confused input visitor for BlockdevOptions,
  and the output visitor.
  This may not always work but I hope it usually will.  Fingers crossed.
  (At least it won’t make things worse.)

- Tests, tests, tests.


(Yes, I know that “In this series tests, tests, tests.” is not a
 sentence.)


v4:
- Drop the default-variant stuff and replace it by a more general
  concept of allowing default values for all QAPI struct members


git backport-diff against v3:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/14:[down] 'qapi: Parse numeric values'
002/14:[down] 'qapi: Move to_c_string() to common.py'
003/14:[down] 'qapi: Introduce default values for struct members'
004/14:[down] 'qapi: Allow optional discriminators'
005/14:[down] 'qapi: Document default values for struct members'
006/14:[down] 'test-qapi: Print struct members' default values'
007/14:[down] 'tests: Test QAPI default values for struct members'
008/14:[0044] [FC] 'tests: Add QAPI optional discriminator tests'
009/14:[0009] [FC] 'qapi: Formalize qcow2 encryption probing'
010/14:[0005] [FC] 'qapi: Formalize qcow encryption probing'
011/14:[0014] [FC] 'block: Try to create well typed json:{} filenames'
012/14:[----] [--] 'iotests: Test internal option typing'
013/14:[----] [--] 'iotests: qcow2's encrypt.format is now optional'
014/14:[down] 'block: Make use of QAPI defaults'


Max Reitz (14):
  qapi: Parse numeric values
  qapi: Move to_c_string() to common.py
  qapi: Introduce default values for struct members
  qapi: Allow optional discriminators
  qapi: Document default values for struct members
  test-qapi: Print struct members' default values
  tests: Test QAPI default values for struct members
  tests: Add QAPI optional discriminator tests
  qapi: Formalize qcow2 encryption probing
  qapi: Formalize qcow encryption probing
  block: Try to create well typed json:{} filenames
  iotests: Test internal option typing
  iotests: qcow2's encrypt.format is now optional
  block: Make use of QAPI defaults

 docs/devel/qapi-code-gen.txt                  |  81 +++++-
 tests/Makefile.include                        |  17 +-
 qapi/block-core.json                          | 180 +++++++++-----
 qapi/introspect.json                          |   9 +-
 tests/qapi-schema/bad-type-int.json           |   1 -
 tests/qapi-schema/enum-int-member.json        |   1 -
 ...l-discriminator-invalid-specification.json |  11 +
 ...on-optional-discriminator-no-default.json} |   5 +-
 tests/qapi-schema/qapi-schema-test.json       |  38 +++
 .../struct-member-alternate-default.json      |  10 +
 ...struct-member-bool-wrong-default-type.json |   3 +
 .../struct-member-enum-invalid-default.json   |   4 +
 ...struct-member-enum-wrong-default-type.json |   4 +
 .../struct-member-float-invalid-default.json  |   4 +
 ...truct-member-float-wrong-default-type.json |   3 +
 .../struct-member-int-wrong-default-type.json |   3 +
 .../struct-member-int8-erange-default.json    |   3 +
 .../struct-member-list-nonempty-default.json  |   4 +
 .../struct-member-non-optional-default.json   |   3 +
 .../struct-member-null-default.json           |   6 +
 .../struct-member-str-wrong-default-type.json |   3 +
 .../struct-member-uint8-erange-default.json   |   3 +
 .../struct-member-uint8-negative-default.json |   3 +
 block.c                                       |  68 ++++-
 block/file-posix.c                            |   9 -
 block/file-win32.c                            |   8 +-
 block/parallels.c                             |   6 +-
 block/qcow2.c                                 |  39 +--
 block/qed.c                                   |   3 -
 block/sheepdog.c                              |   3 -
 block/vdi.c                                   |   3 -
 block/vhdx.c                                  |  28 +--
 block/vpc.c                                   |   3 -
 blockdev.c                                    | 182 +++-----------
 monitor/hmp-cmds.c                            |  27 +-
 monitor/qmp-cmds.c                            |   3 +-
 scripts/qapi/commands.py                      |   2 +-
 scripts/qapi/common.py                        | 232 ++++++++++++++++--
 scripts/qapi/doc.py                           |  20 +-
 scripts/qapi/introspect.py                    |   8 +-
 scripts/qapi/types.py                         |   2 +-
 scripts/qapi/visit.py                         |  38 ++-
 tests/qapi-schema/bad-type-int.err            |   2 +-
 tests/qapi-schema/enum-int-member.err         |   2 +-
 ...al-discriminator-invalid-specification.err |   1 +
 ...-discriminator-invalid-specification.exit} |   0
 ...l-discriminator-invalid-specification.out} |   0
 ...nion-optional-discriminator-no-default.err |   1 +
 ...ion-optional-discriminator-no-default.exit |   1 +
 ...nion-optional-discriminator-no-default.out |   0
 .../flat-union-optional-discriminator.err     |   1 -
 tests/qapi-schema/leading-comma-list.err      |   2 +-
 tests/qapi-schema/qapi-schema-test.out        |  33 +++
 .../struct-member-alternate-default.err       |   1 +
 .../struct-member-alternate-default.exit      |   1 +
 .../struct-member-alternate-default.out       |   0
 .../struct-member-bool-wrong-default-type.err |   1 +
 ...struct-member-bool-wrong-default-type.exit |   1 +
 .../struct-member-bool-wrong-default-type.out |   0
 .../struct-member-enum-invalid-default.err    |   1 +
 .../struct-member-enum-invalid-default.exit   |   1 +
 .../struct-member-enum-invalid-default.out    |   0
 .../struct-member-enum-wrong-default-type.err |   1 +
 ...struct-member-enum-wrong-default-type.exit |   1 +
 .../struct-member-enum-wrong-default-type.out |   0
 .../struct-member-float-invalid-default.err   |   1 +
 .../struct-member-float-invalid-default.exit  |   1 +
 .../struct-member-float-invalid-default.out   |   0
 ...struct-member-float-wrong-default-type.err |   1 +
 ...truct-member-float-wrong-default-type.exit |   1 +
 ...struct-member-float-wrong-default-type.out |   0
 .../struct-member-int-wrong-default-type.err  |   1 +
 .../struct-member-int-wrong-default-type.exit |   1 +
 .../struct-member-int-wrong-default-type.out  |   0
 .../struct-member-int8-erange-default.err     |   1 +
 .../struct-member-int8-erange-default.exit    |   1 +
 .../struct-member-int8-erange-default.out     |   0
 .../struct-member-list-nonempty-default.err   |   1 +
 .../struct-member-list-nonempty-default.exit  |   1 +
 .../struct-member-list-nonempty-default.out   |   0
 .../struct-member-non-optional-default.err    |   1 +
 .../struct-member-non-optional-default.exit   |   1 +
 .../struct-member-non-optional-default.out    |   0
 .../struct-member-null-default.err            |   1 +
 .../struct-member-null-default.exit           |   1 +
 .../struct-member-null-default.out            |   0
 .../struct-member-str-wrong-default-type.err  |   1 +
 .../struct-member-str-wrong-default-type.exit |   1 +
 .../struct-member-str-wrong-default-type.out  |   0
 .../struct-member-uint8-erange-default.err    |   1 +
 .../struct-member-uint8-erange-default.exit   |   1 +
 .../struct-member-uint8-erange-default.out    |   0
 .../struct-member-uint8-negative-default.err  |   1 +
 .../struct-member-uint8-negative-default.exit |   1 +
 .../struct-member-uint8-negative-default.out  |   0
 tests/qapi-schema/test-qapi.py                |   8 +-
 tests/qemu-iotests/059.out                    |   2 +-
 tests/qemu-iotests/087                        |  65 +++--
 tests/qemu-iotests/087.out                    |  26 +-
 tests/qemu-iotests/089                        |  25 ++
 tests/qemu-iotests/089.out                    |   9 +
 tests/qemu-iotests/099.out                    |   4 +-
 tests/qemu-iotests/110.out                    |   2 +-
 tests/qemu-iotests/198.out                    |   4 +-
 104 files changed, 915 insertions(+), 384 deletions(-)
 create mode 100644 
tests/qapi-schema/flat-union-optional-discriminator-invalid-specification.json
 rename tests/qapi-schema/{flat-union-optional-discriminator.json => 
flat-union-optional-discriminator-no-default.json} (68%)
 create mode 100644 tests/qapi-schema/struct-member-alternate-default.json
 create mode 100644 tests/qapi-schema/struct-member-bool-wrong-default-type.json
 create mode 100644 tests/qapi-schema/struct-member-enum-invalid-default.json
 create mode 100644 tests/qapi-schema/struct-member-enum-wrong-default-type.json
 create mode 100644 tests/qapi-schema/struct-member-float-invalid-default.json
 create mode 100644 
tests/qapi-schema/struct-member-float-wrong-default-type.json
 create mode 100644 tests/qapi-schema/struct-member-int-wrong-default-type.json
 create mode 100644 tests/qapi-schema/struct-member-int8-erange-default.json
 create mode 100644 tests/qapi-schema/struct-member-list-nonempty-default.json
 create mode 100644 tests/qapi-schema/struct-member-non-optional-default.json
 create mode 100644 tests/qapi-schema/struct-member-null-default.json
 create mode 100644 tests/qapi-schema/struct-member-str-wrong-default-type.json
 create mode 100644 tests/qapi-schema/struct-member-uint8-erange-default.json
 create mode 100644 tests/qapi-schema/struct-member-uint8-negative-default.json
 create mode 100644 
tests/qapi-schema/flat-union-optional-discriminator-invalid-specification.err
 rename tests/qapi-schema/{flat-union-optional-discriminator.exit => 
flat-union-optional-discriminator-invalid-specification.exit} (100%)
 rename tests/qapi-schema/{flat-union-optional-discriminator.out => 
flat-union-optional-discriminator-invalid-specification.out} (100%)
 create mode 100644 
tests/qapi-schema/flat-union-optional-discriminator-no-default.err
 create mode 100644 
tests/qapi-schema/flat-union-optional-discriminator-no-default.exit
 create mode 100644 
tests/qapi-schema/flat-union-optional-discriminator-no-default.out
 delete mode 100644 tests/qapi-schema/flat-union-optional-discriminator.err
 create mode 100644 tests/qapi-schema/struct-member-alternate-default.err
 create mode 100644 tests/qapi-schema/struct-member-alternate-default.exit
 create mode 100644 tests/qapi-schema/struct-member-alternate-default.out
 create mode 100644 tests/qapi-schema/struct-member-bool-wrong-default-type.err
 create mode 100644 tests/qapi-schema/struct-member-bool-wrong-default-type.exit
 create mode 100644 tests/qapi-schema/struct-member-bool-wrong-default-type.out
 create mode 100644 tests/qapi-schema/struct-member-enum-invalid-default.err
 create mode 100644 tests/qapi-schema/struct-member-enum-invalid-default.exit
 create mode 100644 tests/qapi-schema/struct-member-enum-invalid-default.out
 create mode 100644 tests/qapi-schema/struct-member-enum-wrong-default-type.err
 create mode 100644 tests/qapi-schema/struct-member-enum-wrong-default-type.exit
 create mode 100644 tests/qapi-schema/struct-member-enum-wrong-default-type.out
 create mode 100644 tests/qapi-schema/struct-member-float-invalid-default.err
 create mode 100644 tests/qapi-schema/struct-member-float-invalid-default.exit
 create mode 100644 tests/qapi-schema/struct-member-float-invalid-default.out
 create mode 100644 tests/qapi-schema/struct-member-float-wrong-default-type.err
 create mode 100644 
tests/qapi-schema/struct-member-float-wrong-default-type.exit
 create mode 100644 tests/qapi-schema/struct-member-float-wrong-default-type.out
 create mode 100644 tests/qapi-schema/struct-member-int-wrong-default-type.err
 create mode 100644 tests/qapi-schema/struct-member-int-wrong-default-type.exit
 create mode 100644 tests/qapi-schema/struct-member-int-wrong-default-type.out
 create mode 100644 tests/qapi-schema/struct-member-int8-erange-default.err
 create mode 100644 tests/qapi-schema/struct-member-int8-erange-default.exit
 create mode 100644 tests/qapi-schema/struct-member-int8-erange-default.out
 create mode 100644 tests/qapi-schema/struct-member-list-nonempty-default.err
 create mode 100644 tests/qapi-schema/struct-member-list-nonempty-default.exit
 create mode 100644 tests/qapi-schema/struct-member-list-nonempty-default.out
 create mode 100644 tests/qapi-schema/struct-member-non-optional-default.err
 create mode 100644 tests/qapi-schema/struct-member-non-optional-default.exit
 create mode 100644 tests/qapi-schema/struct-member-non-optional-default.out
 create mode 100644 tests/qapi-schema/struct-member-null-default.err
 create mode 100644 tests/qapi-schema/struct-member-null-default.exit
 create mode 100644 tests/qapi-schema/struct-member-null-default.out
 create mode 100644 tests/qapi-schema/struct-member-str-wrong-default-type.err
 create mode 100644 tests/qapi-schema/struct-member-str-wrong-default-type.exit
 create mode 100644 tests/qapi-schema/struct-member-str-wrong-default-type.out
 create mode 100644 tests/qapi-schema/struct-member-uint8-erange-default.err
 create mode 100644 tests/qapi-schema/struct-member-uint8-erange-default.exit
 create mode 100644 tests/qapi-schema/struct-member-uint8-erange-default.out
 create mode 100644 tests/qapi-schema/struct-member-uint8-negative-default.err
 create mode 100644 tests/qapi-schema/struct-member-uint8-negative-default.exit
 create mode 100644 tests/qapi-schema/struct-member-uint8-negative-default.out

-- 
2.21.0