Re: [PATCH v4 13/32] libxl_qmp: Connect to QMP socket

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Aug 03, 2018 at 02:54:20PM +0100, Anthony PERARD wrote:
> On Thu, Aug 02, 2018 at 11:35:53AM +0200, Roger Pau Monné wrote:
> > On Fri, Jul 27, 2018 at 03:05:55PM +0100, Anthony PERARD wrote:
> > > +typedef enum {
> > > +    qmp_state_disconnected = 1,
> > > +    qmp_state_connecting,
> > > +    qmp_state_greeting,
> > > +    qmp_state_capability_negociation,
> > > +    qmp_state_connected,
> > > +} libxl__qmp_state;
> > > +
> > 
> > I think this should be declared in libxl_types_internal.idl?
> 
> I don't know, I kind of wanted the enum to be contained in libxl_qmp.c,
> but then I couldn't use the enum type in the struct here.
> 
> Also the idl provides more than needed (conversion to string) and make
> the names more verbose, by adding libxl__ prefix.

Hm, yes. libxl_types_internal.idl doesn't seem to be used much. I will
leave this to the maintainers because it's not clear to me when a
declaration should live in the IDL or the private header directly.

> > And (maybe) a description of each state would be helpful for future
> > reference?
> 
> Yes, but not in libxl_internal.h, the description should be in
> libxl_qmp.c as this qmp_state is not part of the API.

I would place the description together with the declaration of the
type, or else it's very easy for things to get out of sync.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel