Re: [Qemu-devel] [PATCH v2] qtest: ask endianness of the target in qtest_init()

[Greg Kurz <groug@kaod.org>]

On Fri, 7 Oct 2016 14:56:26 +0200
Laurent Vivier <lvivier@redhat.com> wrote:

> On 07/10/2016 14:52, Peter Maydell wrote:
> > On 7 October 2016 at 13:45, Greg Kurz <groug@kaod.org> wrote:  
> >> On Fri, 7 Oct 2016 13:31:10 +0100
> >> Peter Maydell <peter.maydell@linaro.org> wrote:
> >>  
> >>> On 7 October 2016 at 13:27, Greg Kurz <groug@kaod.org> wrote:  
> >>>> Indeed but my suggestion is to open code this in qvirtio_is_big_endian(),
> >>>> and even rename QTestState::big_endian to virtio_big_endian to make it
> >>>> really obvious it should not be used elsewhere.
> >>>>
> >>>> I now remember this is what I was resolutely suggested to do in
> >>>> include/qom/cpu.h at the time we started to support ppc64le:
> >>>>
> >>>>     bool (*virtio_is_big_endian)(CPUState *cpu);  
> >>>
> >>> Not really the same thing though -- virtio_is_big_endian
> >>> in QEMU is indeed used only in virtio, because it makes
> >>> dubious use of the internals of the CPU state. The
> >>> equivalent of this proposed qtest function is the #define
> >>> TARGET_BIG_ENDIAN, which is global to all of QEMU and
> >>> reasonably widely used (because it's not a property of
> >>> the CPU's internals).
> >>>  
> >>
> >> Indeed but is it expected to be used in other tests than
> >> virtio ?  
> > 
> > Well, that's where we came in.
> > 
> > Personally I'd rather see this patch purely fix the current
> > rather dodgy implementation of the existing qtest_big_endian()
> > function, which seems to be non-controversial, rather than
> > getting bogged down too much in the questions about what the
> > function name should be and how widely it should be used, etc.  
> 
> I'd rather too..
> 

Fair enough.

> And I can rework this part later, as I've a series to enable virtio
> tests for SPAPR.
> 
> So if v2 covers all non virtio naming space issues, is it acceptable as-is?
> 

What I said with v1 still stands: this is an improvement over what we currently
have.

Reviewed-by: Greg Kurz <groug@kaod.org>

> Thanks,
> Laurent

Cheers.

--
Greg