On Wed, Aug 01, 2018 at 01:10:08PM +0100, Daniel P. Berrangé wrote:
> On Wed, Aug 01, 2018 at 01:41:48PM +0200, Bjoern Walk wrote:
> > Daniel P. Berrangé <berrange@xxxxxxxxxx> [2018-08-01, 11:51AM +0100]:
> > > On Tue, Jul 31, 2018 at 03:54:43PM +0200, Bjoern Walk wrote:
> > > > Bjoern Walk <bwalk@xxxxxxxxxxxxx> [2018-07-31, 03:16PM +0200]:
> > > > > I have not yet had the time to figure out what goes wrong, any ideas are welcome.
> > > >
> > > > Ah, classic. The mocked virRandomBytes function is not endian-agnostic,
> > > > generating a different interface name as expected by the test.
> > >
> > > I'm not seeing why virRandomBytes is affected by endian-ness. It is simply
> > > populating an array of bytes, so there's no endin issues to consider there.
> >
> > Ahem, we are writing linearly to a byte array, of course this is
> > dependend on the endianness of the architecture. The actual problem is
> > that the mocked version does _not_ provide a random value, but a
> > deterministic one, which is byte order reversed on big endianness.
>
> There's no concept of reversed byte order when you're accessing an
> array of bytes. If you write elements 0, 1, 2, ...7, etc and then
> the caller reads elements 0, 1,2, ..., 7 everything is fine.
>
> Endianness only comes into play if you take that array of bytes
> and then cast/assign it to a larger type were endianess is
> relevant (eg int16/int32/int64)
>
> eg
>
> char bytes[8];
>
> virRandomBytes(bytes, 8);
>
> uint64_t val = (uint64_t)bytes;
>
> the problem in such a case isn't virRandomBytes, it is the cast
> to the larger sized integer type.
>
> > > Can you elaborate on the actual error messages you are getting from the
> > > tests, and what aspect makes you think virRandomBytes is the problem ?
> >
> > The name of the interface is wrong compared to what is explicitly
> > expected in the test case:
> >
> > 200 if (virAsprintf(&temp_ifacename,
> > (gdb)
> > 205 VIR_DEBUG("Attempting to create interface '%s' with IQN '%s'",
> > (gdb) p temp_ifacename
> > $1 = 0x1014320 "libvirt-iface-04050607"
> >
> > > Seems more likely that it is something higher up the call stack.
>
> That looks like it uses virRandomBits rather than virRandomBytes.
Yes, it is virRandomBits that is the problem - it is taking a uint64_t
variable and casting it to a "unsigned char *", which is not an
endian safe thing todo.
We need to rewrite virRandomBits to do
char val[8];
virRandomBytes(val, sizeof(val));
uint64_t ret =
val[0] |
((uint64_t)val[1]) << 8 |
((uint64_t)val[2]) << 16 |
((uint64_t)val[3]) << 24 |
((uint64_t)val[4]) << 32 |
((uint64_t)val[5]) << 40 |
((uint64_t)val[6]) << 48 |
((uint64_t)val[7]) << 56 |
return ret & ((1ULL << nbits) -1);
to make it endiansafe.
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 :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
