qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Huth <thuth@redhat.com>
To: "Greg Kurz" <groug@kaod.org>, "Cédric Le Goater" <clg@kaod.org>
Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
	David Gibson <david@gibson.dropbear.id.au>,
	Alexey Kardashevskiy <aik@ozlabs.ru>
Subject: Re: [Qemu-devel] [PATCH 1/4] spapr: remove irq_hint parameter from spapr_irq_alloc()
Date: Mon, 28 May 2018 08:17:47 +0200	[thread overview]
Message-ID: <82ee8b24-91d7-9396-82b7-be37200cfacd@redhat.com> (raw)
In-Reply-To: <20180525160249.7cb0a5bc@bahia.lan>

On 25.05.2018 16:02, Greg Kurz wrote:
> On Fri, 18 May 2018 18:44:02 +0200
> Cédric Le Goater <clg@kaod.org> 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.

 Thomas

  reply	other threads:[~2018-05-28  6:17 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-18 16:44 [Qemu-devel] [PATCH 0/4] spapr: generic IRQ frontend Cédric Le Goater
2018-05-18 16:44 ` [Qemu-devel] [PATCH 1/4] spapr: remove irq_hint parameter from spapr_irq_alloc() Cédric Le Goater
2018-05-25 14:02   ` Greg Kurz
2018-05-28  6:17     ` Thomas Huth [this message]
2018-05-28  7:06       ` Cédric Le Goater
2018-05-28  7:18         ` [Qemu-devel] [Qemu-ppc] " Thomas Huth
2018-05-28  9:20           ` Cédric Le Goater
2018-05-28 12:09             ` Greg Kurz
2018-05-28 13:33               ` Cédric Le Goater
2018-06-05  3:34         ` [Qemu-devel] " David Gibson
2018-06-05  6:41           ` Cédric Le Goater
2018-06-13  4:22             ` David Gibson
2018-06-13  7:24               ` Cédric Le Goater
2018-06-14  3:46                 ` David Gibson
2018-06-14 13:26                   ` Cédric Le Goater
2018-06-02  9:19       ` Cédric Le Goater
2018-06-04  6:05         ` Cédric Le Goater
2018-06-02  9:10   ` Cédric Le Goater
2018-05-18 16:44 ` [Qemu-devel] [PATCH 2/4] sparp_pci: simplify how the PCI LSIs are allocated Cédric Le Goater
2018-05-26  9:40   ` Greg Kurz
2018-06-05  3:44     ` David Gibson
2018-06-05  6:31       ` Cédric Le Goater
2018-06-13  4:27         ` David Gibson
2018-06-13  7:26           ` Cédric Le Goater
2018-05-18 16:44 ` [Qemu-devel] [PATCH 3/4] spapr: introduce a generic IRQ frontend to the machine Cédric Le Goater
2018-05-28 14:27   ` Greg Kurz
2018-06-13  5:00   ` David Gibson
2018-06-13  7:44     ` Cédric Le Goater
2018-05-18 16:44 ` [Qemu-devel] [PATCH 4/4] spapr: introduce a new IRQ backend using fixed IRQ number ranges Cédric Le Goater
2018-05-28 15:18   ` Greg Kurz
2018-05-28 15:42     ` Cédric Le Goater
2018-05-29 12:51     ` Cédric Le Goater

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=82ee8b24-91d7-9396-82b7-be37200cfacd@redhat.com \
    --to=thuth@redhat.com \
    --cc=aik@ozlabs.ru \
    --cc=clg@kaod.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).