Re: [Qemu-devel] [PATCH 08/32] kvm/openpic: in-kernel mpic support

[Andreas Färber]

Am 01.07.2013 01:18, schrieb Andreas Färber:
> Am 01.07.2013 01:01, schrieb Alexander Graf:
>>
>> On 30.06.2013, at 08:13, Andreas Färber wrote:
>>
>>> Am 30.06.2013 03:44, schrieb Alexander Graf:
>>>> From: Scott Wood <address@hidden>
>>>>
>>>> Enables support for the in-kernel MPIC that thas been merged into the
>>>> KVM next branch.  This includes irqfd/KVM_IRQ_LINE support from Alex
>>>> Graf (along with some other improvements).
>>>>
>>>> Note from Alex regarding kvm_irqchip_create():
>>>>
>>>>  On x86, one would call kvm_irqchip_create() to initialize an
>>>>  in-kernel interrupt controller.  That function then goes ahead and
>>>>  initializes global capability variables as well as the default irq
>>>>  routing table.
>>>>
>>>>  On ppc, we can't call kvm_irqchip_create() because we can have
>>>>  different types of interrupt controllers.  So we want to do all the
>>>>  things that function would do for us in the in-kernel device init
>>>>  handler.
>>>>
>>>> Signed-off-by: Scott Wood <address@hidden>
>>>> [agraf: squash in kvm_irqchip_commit_routes patch, fix non-kvm build]
>>>> Signed-off-by: Alexander Graf <address@hidden>
>>>> ---
>>>> default-configs/ppc-softmmu.mak   |   1 +
>>>> default-configs/ppc64-softmmu.mak |   1 +
>>>> hw/intc/Makefile.objs             |   1 +
>>>> hw/intc/openpic_kvm.c             | 252 
>>>> ++++++++++++++++++++++++++++++++++++++
>>>> hw/ppc/e500.c                     |  79 +++++++++++-
>>>> include/hw/ppc/openpic.h          |   2 +-
>>>> target-ppc/kvm-stub.c             |   6 +
>>>> 7 files changed, 336 insertions(+), 6 deletions(-)
>>>> create mode 100644 hw/intc/openpic_kvm.c
>>>
>>> I had objected to the subject
>>
>> What's wrong with the subject? I don't find it misleading. I don't remember 
>> we ever had strong ruling on subject lines. In fact, I usually format mine 
>> completely differently.
> 
> ...and I have complained about that often enough, which you are ignoring.
> 
>>> , and this patch is not bisectable since
>>> you didn't squash my ppcemb-softmmu build fix. Please do.
>>
>> Please send build fixes separately from QOM refactoring. The patch as a 
>> whole is way too big to get squashed into the original patch. I'll extract 
>> the build fix and pluck it up manually this time, but please keep things 
>> separate next time around.
> 
> No, this was not a refactoring of in-tree code, it was a fixup that
> Scott should have done. Next time review and test properly, then it
> wouldn't have made it into your tree in the first place and I wouldn't
> have needed to make my hands dirty with that crap.

Maybe it was late for both of us and it came over a bit harsh, but it
seems that you were neither listening to nor reading review comments and
just blindly applying patches to ppc-next first-come, first-served.

I clearly replied to Scott's patch indicating it breaks the build, and
my subject had both "fixup" and "build issues" in the subject line and
it was --in-reply-to the offending patch. If that is still a surprise to
you, I really don't know what else to do...

You have nitpicks about my OpenBIOS or Alexey's sPAPR patches yourself,
and my nack was easy if not trivial to resolve with a v3 or in-queue
fixup, so I really don't see it as unreasonable to ask for fixing up a
patch on a rebasing queue long before the PULL was sent.

Andreas

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