On Wed, May 11, 2022 at 08:38:04AM -0700, Andrea Bolognani wrote: > On Tue, May 10, 2022 at 01:51:05PM +0100, Daniel P. Berrangé wrote: > > 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 honestly looks pretty unwieldy.
It isn't all that more verbose than without the versions - just a single struct wrapper. > > If the application already knows it's targeting a specific version of > the QEMU API, which for the above code to make any sense it will have > to, couldn't it do something like > > import qemu .../qemu/v700 > > at the beginning of the file and then use regular old > > cmd := qemu.BlockResizeArguments{ > NodeName: nodeName, > Size: size, > } > > instead? This would lead to a situation where every struct is duplicated for every version, even though 90% of the time they'll be identical across multiple versions. This is not very ammenable to the desire to be able to dynamically choose per-command which version you want based on which version of QEMU you're connected to. ie var cmd Command if qmp.HasVersion(qemu.Version(7, 0, 0)) { cmd = BlockResizeArguments{ V700: &BlockResizeArguments700{ NodeName: node, Size: 1 * GiB } } } else { cmd = BlockResizeArguments{ V520: &BlockResizeArguments520{ Device: dev, Size: 1 * GiB } } } And of course the HasVersion check is going to be different for each command that matters. Having said that, this perhaps shows the nested structs are overkill. We could have var cmd Command if qmp.HasVersion(qemu.Version(7, 0, 0)) { cmd = &BlockResizeArguments700{ NodeName: node, Size: 1 * GiB } } else { cmd = &BlockResizeArguments520{ Device: dev, Size: 1 * GiB } } If there was some need for common handling of the different versioned variants, we could still have a 'BlockResizeArguments' that has a field per version, as an optional thing. Or have a BlockResizeArguments interface, implemented by each version </hand-wavey> 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 :|