[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [v5 PATCH 0/2] apic: bump emulated lapic version to 0x1
From: |
Gabriel L. Somlo |
Subject: |
Re: [Qemu-devel] [v5 PATCH 0/2] apic: bump emulated lapic version to 0x14 |
Date: |
Mon, 5 May 2014 14:08:21 -0400 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, May 05, 2014 at 07:38:58PM +0200, Andreas F?rber wrote:
> 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. :)
You're right; the dependency was mentioned in the v4 cover letter, but
in retrospect it makes perfect sense I should have kept appending to that
content instead of using it as a place to reply to the last person who
commented on the previous version :)
Re. all that stuff you said about how to handle acked-by and
reviewed-by replies, is there a good spot where that process is
documented ? I noticed you all have a protocol in place for dealing
with that, but this is the first time I had a chance to screw it
up myself :) Googling around, I found this:
https://www.kernel.org/doc/Documentation/SubmittingPatches
Does QEMU have its own, or is this what I need for future reference ?
> 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,
I get no compiler warnings, and adjacent case branches also assign
other 8-bit values to the 32-bit "val" variable w/o a cast, so I think
we're OK.
>
> Reviewed-by: Andreas F?rber <address@hidden>
Thanks much,
--Gabriel