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

Re: [Xen-devel] [PATCH 2/2] libxl: fix UUID usage on FreeBSD



On Fri, 2016-01-15 at 16:11 +0100, Roger Pau Monnà wrote:
> El 15/01/16 a les 11.26, Ian Campbell ha escrit:
> > On Tue, 2016-01-12 at 14:14 +0100, Roger Pau Monne wrote:
> > > libxl makes the assumtion that libxl_uuid == uuid_t,
> > 
> > ("assumption")
> > 
> > > Âand that uuid_t can be
> > > freely used as a byte array. This is not true on FreeBSD (and NetBSD
> > > too, not sure about other BSD UUID implementations), where the
> > > internals
> > > of
> > > uuid don't match what libxl expects as a byte array because of
> > > endianness
> > > issues.
> > > 
> > > Fix this by converting the libxl_uuid type to a struct with an
> > > internal
> > > uuid_t field and a byte-array. Also introduce a new function that
> > > should
> > > be
> > > used in order to load a byte array into a uuid_t struct.
> > 
> > Do we really need to keep both the uuid_t and the byte-array
> > representation
> > around? It looks to me as if we only really need the byte-array form,
> > which
> > might then involve changing various uses of uuid_* internally to just
> > be
> > mem*.
> 
> Yes, we can remove the uuid_t from libxl_uuid, but this is AFAICT a
> structure that belongs to the stable API.

Oh b*m, so it is.

>  My current change keeps the
> same layout by turning the union into a struct, but without changing the
> fields.

The danger with your change is that the two halves can now get out of sync.

Neither xl nor libvirt actually poke into the contents of the struct at
all. So I wonder if we can get away with deprecating it?

Or can we get rid of uuid_raw (which is not touched, and is less likely to
be given that it is only on a subset of platforms) and have libxl
_internal_ stuff convert to a byte array.

The problem there I guess is that would involve changing the semantics of
libxl_uuid_bytearray{,_const} (since the result would now need to be freed,
since both returns a static buffer, so who knows what the const distinction
was supposed to be for!).

Hrm, we've certainly painted ourselves into a corner with this one :-/

If we do go with keeping both the uuid_t and the raw array then we should
clearly mark the uuid_t one as the canonical copy (the raw one essentially
becomes a scratch space used to facilitate the provision of the broken
libxl_uuid_bytearray* interfaces).

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.