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. -- Andrea Bolognani / Red Hat / Virtualization