qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH v1 0/8] qapi: add generator for Golang interface


From: Andrea Bolognani
Subject: Re: [RFC PATCH v1 0/8] qapi: add generator for Golang interface
Date: Wed, 11 May 2022 09:22:36 -0700

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]