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

From: Wei Liu <wei.liu2@citrix.com>
To: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Anthony PERARD <anthony.perard@citrix.com>,
	xen-devel@lists.xenproject.org,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH v4 13/32] libxl_qmp: Connect to QMP socket
Date: Tue, 21 Aug 2018 09:22:25 +0100	[thread overview]
Message-ID: <20180821082225.64eui7pryjfesrqv@citrix.com> (raw)
In-Reply-To: <20180803140354.nsw6ofwb34lcaqvi@mac>

On Fri, Aug 03, 2018 at 04:03:54PM +0200, Roger Pau Monné wrote:
> 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.

I think in this case it is better to use the internal header. The type
introduced here doesn't need all the fancy stuff generated by the IDL
infrastructure.

Wei.

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