Ping On 24.06.19 19:39, Max Reitz wrote: > 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 >