On Tue, May 10, 2022 at 01:34:03PM +0100, Daniel P. Berrangé wrote: > On Tue, May 10, 2022 at 02:02:56PM +0200, Markus Armbruster wrote: > > > For a minimum viable use case, this doesn't feel all that difficult, as > > > conceptually instead of deleting the field from QAPI, we just need to > > > annotate it to say when it was deleted from the QEMU side. The QAPI > > > generator for internal QEMU usage, can omit any fields annotated as > > > deleted in QAPI schema. The QAPI generator for external app usage, > > > can (optionally) be told to include deleted fields ranging back to > > > a given version number. So apps can chooses what degree of compat > > > they wish to retain. > > > > Consider this evolution of command block_resize > > To help us understand, I'll illustrate some possible interfaces > in both Go and Python, since that covers dynamic and static > languages > > > * Initially, it has a mandatory argument @device[*]. > > Python definition: > > def block_resize(device, size) > > Caller: > > block_resize('dev0', 1*GiB) > > > Golang definition > > type BlockResizeCommand struct { > Device string > Size int > } > > Caller > > cmd := &BlockResizeCommand{ > Device: "dev0", > Size: 1 * GiB, > } > > > * An alternative way to specify the command's object emerges: new > > argument @node-name. Both old @device and new @node-name become > > optional, and exactly one of them must be specified. This is commit > > 3b1dbd11a6 "qmp: Allow block_resize to manipulate bs graph nodes." > > Python definition. Tricky, as non-optional params must be before > optional params, but size is naturally the last arg. One option > is to pointlessly mark 'size' as optional > > def block_resize(device=None, node_name=None, size=None) > > Caller > > block_resize(device="dev0", size=1*GiB) > block_resize(node_name="devnode0", size=1*GiB) > > > In golang definition > > type BlockResizeArguments struct { > Device string > NodeName string > Size int > } > > Caller choice of > > cmd := &BlockResizeCommand{ > Device: "dev0", > Size: 1 * GiB, > } > > cmd := &BlockResizeCommand{ > NodeName: "devnode0", > Size: 1 * GiB, > } > > > Neither case can easily prevent passing Device and NodeName > at same time. > > > * At some future date, the old way gets deprecated: argument @device > > acquires feature @deprecated. > > Ok, no change needed to the APIs in either case. Possibly have > code emit a warning if a deprecated field is set. > > > * Still later, the old way gets removed: @device is deleted, and > > @node-name becomes mandatory. > > Again no change needed to APIs, but QEMU will throw back an > error if the wrong one is used. > > > What is the proper version-spanning interface? > > > > I figure it's both arguments optional, must specify the right one for > > the version of QEMU actually in use. This spans versions, but it fails > > to abstract from them. > > Yep, I think that's inevitable in this scenario. THe plus side > is that apps that want to span versions can do so. The downside > is that apps that don't want smarts to span version, may loose > compile time warnings about use of the now deleted field.
Having said that, a different way to approach the problem is to expose the versioning directly in the generated code. Consider a QAPI with versioning info about the fields { 'command': 'block_resize', 'since': '5.0.0', 'data': { 'device': ['type': 'str', 'until': '5.2.0' ], '*device': ['type': 'str', 'since': '5.2.0', 'until': '7.0.0' ], '*node-name': ['type': 'str', 'since': '5.2.0', 'until: '7.0.0' ], 'node-name': ['type': 'str', 'since': '7.0.0' ], 'size': 'int' } } Meaning * Introduced in 5.0.0, with 'device' mandatory * In 5.2.0, 'device' becomes optional, with optional 'node-name' as alternative * In 7.0.0, 'device' is deleted, and 'node-name' becomes mandatory Now consider the Go structs In 5.0.0 we can generate: type BlockResizeArguments struct { V500 *BlockResizeArguments500 } type BlockResizeArgumentsV1 struct { Device string Size int } app can use dev := "dev0" cmd := BlockResizeArguments{ V500: &BlockResizeArguments500{ Device: dev, Size: 1 * GiB } } In 5.2.0 we can now generate type BlockResizeArguments struct { V500 *BlockResizeArgumentsV500 V520 *BlockResizeArgumentsV520 } type BlockResizeArgumentsV500 struct { Device string Size int } type BlockResizeArgumentsV520 struct { Device *string NodeName *string Size int } App can use the same as before, or switch to one of dev := "dev0" cmd := BlockResizeArguments{ V520: &BlockResizeArguments520{ Device: &dev, Size: 1 * GiB } } or node := "nodedev0" cmd := BlockResizeArguments{ V520: &BlockResizeArguments520{ NodeName: &node, Size: 1 * GiB } } In 7.0.0 we can now generate type BlockResizeArguments struct { V500 *BlockResizeArgumentsV500 V520 *BlockResizeArgumentsV520 V700 *BlockResizeArgumentsV700 } type BlockResizeArgumentsV500 struct { Device string Size int } type BlockResizeArgumentsV520 struct { Device *string NodeName *string Size int } type BlockResizeArgumentsV700 struct { NodeName string Size int } App can use the same as before, or switch to node := "nodedev0" cmd := BlockResizeArguments{ V700: &BlockResizeArguments700{ NodeName: node, Size: 1 * GiB } } This kind of per-command/type versioning is not uncommon when defining API protocols/interfaces. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|