Re: [Qemu-devel] [PATCH RFC 4/4] Initial implementation of vGICv3.

[Christoffer Dall <christoffer.dall@linaro.org>]

Hi Pavel,

On Fri, May 22, 2015 at 07:57:13PM +0300, Pavel Fedin wrote:
>  Hi!
> 
> > Looks GICv3 common class currently miss this security_extn field +
> > parent_fiq so it does not compile without changes. Or did I miss something?
> 
>  Just throw this if(...) away. It's my fault. Actually i have rebased Shlomo's patches on
> yesterday's master, and during this i added parent_fiq[] to GICv3, because without filling
> these in the thing stopped working due to recent GICv2 code changes. I needed to fix the
> problem quickly, so i copied initializing parent_fiq together with security_extn field
> from v2 code. But, this makes no sense because security_extn is never initialized, it is
> not even assigned property name. So just remove this small fragment, it's not needed now.
>  I have fixed this in my working tree 2 hours ago. parent_fiq is declared as:
> --- cut ---
> qemu_irq parent_fiq[GICV3_NCPU];
> --- cut ---
> 

I think based on this comment that I can safely suggest a bit of advise
regarding sending patches to the QEMU and Linux lists:

Be as careful as you can, properly test your code, review your own
commit messages, run spell check on them, and try to look at what you
send out through the eyes of someone who has never seen this code
before.

Review resources are really scarse in these projects and almost all
maintainers are overloaded, so we all have to work together and make
sure we don't flood the lists unwarranted.

It is good to re-spin quickly so people don't loose context, but if you
are fixing things up in a matter of hours and sending out new revisions,
you are moving too fast.

Turn-around time is days/weeks in the best cases, so better to take a
few extra days to thoroughly test a patch series.  The exception is to
illustrate a general idea or design, in which case clearly marking the
series as an RFC and noting what people should look at and what people
should ignore is key.

Hope this makes sense,
-Christoffer