Hi, On Wed, May 11, 2022 at 04:50:35PM +0100, Daniel P. Berrangé wrote: > 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 > } > }
The else block would be wrong in versions above 7.0.0 where block_resize changed. There will be a need to know for a specific Type if we are covered with latest qemu/qapi-go or not. Not yet sure how to address that, likely we will need to keep the information that something has been added/changed/removed per version per Type in qapi-go... Still, I think the above proposal is a good compromise to make.. > 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> Cheers, Victor
signature.asc
Description: PGP signature