Re: [Xen-devel] [PATCH v4 1/7] libxl: add definition of INVALID_DOMID to the API

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Jan 31, 2020 at 10:31:49AM +0000, Durrant, Paul wrote:
> > -----Original Message-----
> > From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of
> > Roger Pau Monné
> > Sent: 22 January 2020 14:53
> > To: Durrant, Paul <pdurrant@xxxxxxxxxxxx>
> > Cc: Anthony PERARD <anthony.perard@xxxxxxxxxx>; xen-
> > devel@xxxxxxxxxxxxxxxxxxxx; Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; Wei
> > Liu <wl@xxxxxxx>
> > Subject: Re: [Xen-devel] [PATCH v4 1/7] libxl: add definition of
> > INVALID_DOMID to the API
> > 
> > On Wed, Jan 22, 2020 at 02:44:40PM +0000, Paul Durrant wrote:
> > > Currently both xl and libxl have internal definitions of INVALID_DOMID
> > > which happen to be identical. However, for the purposes of describing
> > the
> > > behaviour of libxl_domain_create_new/restore() it is useful to have a
> > > specified invalid value for a domain id.
> > >
> > > This patch therefore moves the libxl definition from libxl_internal.h to
> > > libxl.h and removes the internal definition from xl_utils.h. The
> > hardcoded
> > > '-1' passed back via domcreate_complete() is then updated to
> > INVALID_DOMID
> > > and comment above libxl_domain_create_new/restore() is accordingly
> > > modified.
> > 
> > Urg, it's kind of ugly to add another definition of invalid domid when
> > there's already DOMID_INVALID in the public headers. I guess there's a
> > reason I'm missing for not using DOMID_INVALID instead of introducing
> > a new value?
> > 
> 
> TBH I don't know. As far as xl/libxl goes, domids are uint32_ts so maybe 
> DOMID_INVALID was for some reason not considered appropriate? Bear in mind, 
> this patch is not truly introducing a new value; it's just making something 
> that was internal but identical in both xl and libxl part of the interface.

Hm, OK. It seems quite a mess that libxl uses a 32bit value when the
hypervisor is using a 16bit field, but I guess there's nothing that
can be done at this point to fix this.

Since this will be part of the public interface now, it might make
sense to define it to the same value as DOMID_INVALID (0x7FF4). And
make sure domid values passed to libxl are truncated to 16bits.

Maybe it's not that relevant, but domids passed to libxl would need to
be sanitized in order to make sure Xen's DOMID_INVALID is not treated
as a valid domid value from libxl's PoV. There are also other special
domids that need to be filtered.

> > If so could this be mentioned in the commit message?
> > 
> 
> I'll add a note to the commit comment to point out that this is not changing 
> any value used by the toolstack.

Thanks!

Roger.

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