Re: [RFC PATCH v1 1/8] qapi: golang: Generate qapi's enum types in Go

[Daniel P . Berrangé]

On Tue, May 10, 2022 at 01:28:39PM +0200, Victor Toso wrote:
> Hi,
> 
> On Tue, May 10, 2022 at 12:19:57PM +0100, Daniel P. Berrangé wrote:
> > > Marshalling does error if you try to convert an int that is not
> > > in the range of the enum type.
> > > 
> > > Unmarshalling should not error in this case, but the field ends
> > > up not being set which defaults to 0 (in this case,
> > > ShutdownCauseNone). That could mislead the *actual* reason but
> > > not without a warning which is expected in this case, imho.
> > > 
> > > (I know is not an actual warning, just a Println, but it'll be
> > > fixed in the next iteration)
> > 
> > I don't thinnk we should be emitting warnings/printlns at all
> > in this case though. The app should be able to consume the enum
> > without needing a warning.  If the app wants to validate
> > against a specific set of constants, it can decide to emit a
> > warning if there was a case it can't handle. We shouldn't emit
> > warnings/errors in the unmarshalling step though,as it isn't
> > the right place to decide on such policy.
> 
> I think it is useful to know that, a binary compiled to qapi-go
> 7.0 but running with qemu 7.1 would have some mismatches. It
> could help detect issues (e.g: Why my program doesn't know/show
> the reason for shutdown?).

If apps are actually looking at the shutdown reasons, it is
trivial for the app themselves to log any unexpected reasons.

They should in fact do that regardless, because if someone
re-compiles the app against a new version of the QEMU Go API,
there may be new ShutdownCause's defined that their codes does
not handle. There's no compile time check from Go side that
their code has handled all ShutdownCause values, since there's
no struct enum type in Go.  This again pushes me to towards
the view that the enums should just remain as strings, the use
of integers gives no functional benefit AFAICT, given the lack
of a real enum type in Go.


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 :|