Re: [Qemu-devel] [v5 PATCH 0/2] apic: bump emulated lapic version to 0x14

[Andreas Färber]

Am 05.05.2014 18:21, schrieb Gabriel L. Somlo:
> Andreas,
> 
> On Mon, May 05, 2014 at 06:00:22PM +0200, Andreas F?rber wrote:
>> Objection! .compat_props are being added to the current pc and q35
>> machines rather than introducing pc-i440fx-2.1 and pc-q35-2.1 machines
>> and adding the compat_props for the 2.0 versions only.
>>
>> If 2.1 machines without .compat_props get added and the alias is moved
>> from 2.0 to 2.1, it'll be fine.
> 
> Patch 1/2 (add compat_props placeholder for 2.0) is written to go in
> on top of this patch:
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2014-05/msg00476.html
> 
> which is the one adding the 2.1 machine type and moving the alias.
> 
> 
> The pair of apic patches under discussion here modifies the (by now
> legacy) 2.0 machine type.
> 
> Does that clear it up, or am I still misunderstanding something ?

Yes, with that patch it's okay, you just forgot to mention that
dependency in your cover letter - also a change log from v1 is missing.
Instead of quoting Alex in the cover letter, you should've placed his
Acked-by before your Signed-off-by in the patches he ack'ed - unless you
did major changes there (e.g., uint8_t), in which case it shouldn't be
in the cover letter either. And please use [PATCH v5 n/m] as canonical
ordering. :)

I trust that you have tested and other reviewers have considered no cast
to be necessary for left-hand s->version in the expression now that it's
uint8_t rather than uint32_t? Then,

Reviewed-by: Andreas Färber <address@hidden>

Cheers,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg