On Wed, May 11, 2022 at 09:22:36AM -0700, Andrea Bolognani wrote: > On Wed, May 11, 2022 at 04:50:35PM +0100, Daniel P. Berrangé wrote: > > 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 > > } > > } > > Right, making the decision at the import level would require adding a > level of indirection and make this kind of dynamic logic awkward. > > We shouldn't force users to sprinkle version numbers everywhere > though, especially since different commands will change at different > points in time. It should be possible to do something like > > if !qmp.HasAPI(600) { > panic("update QEMU") > } > > cmd := &BlockResizeArguments600{ // really BlockResizeArguments520 > Device: device, > Size: size, > } > > or even > > if !qmp.HasAPI(qmp.API.Latest) { > panic("update QEMU") > } > > cmd := &BlockResizeArguments{ > NodeName: nodeName, > Size: size, > } > > Should be easy enough to achieve with type aliases.
I guess we would have a single package which does typedef BlockResizeArguments520 BlockResizeArguments; ...for each type... The interaction with API versioning will be tedious though. For the versioned structs we'll be forever back compatible, due to the append-only nature of the versioned struct approach. For the type aliases, we'll be forever breaking compat as at least 1 struct alias is likely to need changing every QEMU release. Might suggest having 2 distinct go modules. A base module which is versioned as a stable API with semver (forever v1.xxx), and an add-on module that depends on base module, which is versioned as an unstable API with semver (forever v0.xxx) 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 :|