Re: [Qemu-ppc] [PATCH 1/4] spapr: remove irq_hint parameter from spapr_irq_alloc()

[Thomas Huth]

On 28.05.2018 09:06, Cédric Le Goater wrote:
> On 05/28/2018 08:17 AM, Thomas Huth wrote:
>> On 25.05.2018 16:02, Greg Kurz wrote:
>>> On Fri, 18 May 2018 18:44:02 +0200
>>> Cédric Le Goater <address@hidden> wrote:
>>>
>>>> This IRQ number hint can possibly be used by the VIO devices if the
>>>> "irq" property is defined on the command line but it seems it is never
>>>> the case. It is not used in libvirt for instance. So, let's remove it
>>>> to simplify future changes.
>>>>
>>>
>>> Setting an irq manually looks a bit anachronistic. I doubt anyone would
>>> do that nowadays, and the patch does a nice cleanup. So this looks like
>>> a good idea.
>> [...]
>>>> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
>>>> index 472dd6f33a96..cc064f64fccf 100644
>>>> --- a/hw/ppc/spapr_vio.c
>>>> +++ b/hw/ppc/spapr_vio.c
>>>> @@ -455,7 +455,7 @@ static void spapr_vio_busdev_realize(DeviceState 
>>>> *qdev, Error **errp)
>>>>          dev->qdev.id = id;
>>>>      }
>>>>  
>>>> -    dev->irq = spapr_irq_alloc(spapr, dev->irq, false, &local_err);
>>>> +    dev->irq = spapr_irq_alloc(spapr, false, &local_err);
>>>
>>> Silently breaking "irq" like this looks wrong. I'd rather officially remove
>>> it first (ie, kill spapr_vio_props, -5 lines in spapr_vio.c).
>>>
>>> Of course, this raises the question of interface deprecation, and it should
>>> theoretically follow the process described at:
>>>
>>> https://wiki.qemu.org/Features/LegacyRemoval#Rules_for_removing_an_interface
>>>
>>> Cc'ing Thomas, our Chief Deprecation Officer, for insights :)
>>
>> The property is a public interface. Just because it's not used by
>> libvirt does not mean that nobody is using it. So yes, please follow the
>> rules and mark it as deprecated first for two release, before you really
>> remove it.
> 
> This "irq" property is a problem to introduce a new static layout of IRQ 
> numbers. It is in complete opposition. 
> 
> Can we keep it as it is for old pseries machine (settable) and ignore it 
> for newer ? Would that be fine ?

I think that would be fine, too. You likely need to keep the settable
IRQs around for the old machines anyway, to make sure that migration of
the old machine types still works, right?

 Thomas