diff mbox

[08/22] target-i386: ioapic: replace FROM_SYSBUS() with QOM type cast

Message ID 1365172636-28628-9-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov April 5, 2013, 2:37 p.m. UTC 
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/ioapic_common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

liguang April 8, 2013, 2:13 a.m. UTC | #1 
This patch should be combined with [PATCH 07/22]

在 2013-04-05五的 16:37 +0200，Igor Mammedov写道：
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/ioapic_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
> index d4aff29..561b987 100644
> --- a/hw/ioapic_common.c
> +++ b/hw/ioapic_common.c
> @@ -59,7 +59,7 @@ static int ioapic_dispatch_post_load(void *opaque, int version_id)
>  
>  static int ioapic_init_common(SysBusDevice *dev)
>  {
> -    IOAPICCommonState *s = FROM_SYSBUS(IOAPICCommonState, dev);
> +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
>      IOAPICCommonClass *info;
>      static int ioapic_no;
>
Igor Mammedov April 8, 2013, 11:32 a.m. UTC | #2 
On Mon, 08 Apr 2013 10:13:28 +0800
li guang <lig.fnst@cn.fujitsu.com> wrote:

> This patch should be combined with [PATCH 07/22]
> 
> 在 2013-04-05五的 16:37 +0200，Igor Mammedov写道：
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  hw/ioapic_common.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
> > index d4aff29..561b987 100644
> > --- a/hw/ioapic_common.c
> > +++ b/hw/ioapic_common.c
> > @@ -59,7 +59,7 @@ static int ioapic_dispatch_post_load(void *opaque, int
> > version_id) 
> >  static int ioapic_init_common(SysBusDevice *dev)
> >  {
> > -    IOAPICCommonState *s = FROM_SYSBUS(IOAPICCommonState, dev);
> > +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
> >      IOAPICCommonClass *info;
> >      static int ioapic_no;
> >  
> 
> 
> 

sure, will do this on the next respin.
Paolo Bonzini April 9, 2013, 11:36 a.m. UTC | #3 
Il 08/04/2013 13:32, Igor Mammedov ha scritto:
> 
>> > This patch should be combined with [PATCH 07/22]
>> > 
>> > 在 2013-04-05五的 16:37 +0200，Igor Mammedov写道：
>>> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> > > ---
>>> > >  hw/ioapic_common.c | 2 +-
>>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>>> > > 
>>> > > diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
>>> > > index d4aff29..561b987 100644
>>> > > --- a/hw/ioapic_common.c
>>> > > +++ b/hw/ioapic_common.c
>>> > > @@ -59,7 +59,7 @@ static int ioapic_dispatch_post_load(void *opaque, int
>>> > > version_id) 
>>> > >  static int ioapic_init_common(SysBusDevice *dev)
>>> > >  {
>>> > > -    IOAPICCommonState *s = FROM_SYSBUS(IOAPICCommonState, dev);
>>> > > +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
>>> > >      IOAPICCommonClass *info;
>>> > >      static int ioapic_no;
>>> > >  
>> > 
>> > 
>> > 
> sure, will do this on the next respin.

Actually no, why?  It's two different files.

Paolo
liguang April 10, 2013, 12:21 a.m. UTC | #4 
在 2013-04-09二的 13:36 +0200，Paolo Bonzini写道：
> Il 08/04/2013 13:32, Igor Mammedov ha scritto:
> > 
> >> > This patch should be combined with [PATCH 07/22]
> >> > 
> >> > 在 2013-04-05五的 16:37 +0200，Igor Mammedov写道：
> >>> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >>> > > ---
> >>> > >  hw/ioapic_common.c | 2 +-
> >>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> >>> > > 
> >>> > > diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
> >>> > > index d4aff29..561b987 100644
> >>> > > --- a/hw/ioapic_common.c
> >>> > > +++ b/hw/ioapic_common.c
> >>> > > @@ -59,7 +59,7 @@ static int ioapic_dispatch_post_load(void *opaque, int
> >>> > > version_id) 
> >>> > >  static int ioapic_init_common(SysBusDevice *dev)
> >>> > >  {
> >>> > > -    IOAPICCommonState *s = FROM_SYSBUS(IOAPICCommonState, dev);
> >>> > > +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
> >>> > >      IOAPICCommonClass *info;
> >>> > >      static int ioapic_no;
> >>> > >  
> >> > 
> >> > 
> >> > 
> > sure, will do this on the next respin.
> 
> Actually no, why?  It's two different files.

because they do the same trivial thing,
do you want some mechanic changes separated?
Paolo Bonzini April 10, 2013, 8:06 a.m. UTC | #5 
Il 10/04/2013 02:21, li guang ha scritto:
>>>>> > >> > 
>>>>> > >> > 
>>> > > sure, will do this on the next respin.
>> > 
>> > Actually no, why?  It's two different files.
> because they do the same trivial thing,
> do you want some mechanic changes separated?

If it makes sense, yes.

Paolo
Igor Mammedov April 10, 2013, 4:12 p.m. UTC | #6 
On Wed, 10 Apr 2013 08:21:17 +0800
li guang <lig.fnst@cn.fujitsu.com> wrote:

> 在 2013-04-09二的 13:36 +0200，Paolo Bonzini写道：
> > Il 08/04/2013 13:32, Igor Mammedov ha scritto:
> > > 
> > >> > This patch should be combined with [PATCH 07/22]
> > >> > 
> > >> > 在 2013-04-05五的 16:37 +0200，Igor Mammedov写道：
> > >>> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > >>> > > ---
> > >>> > >  hw/ioapic_common.c | 2 +-
> > >>> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>> > > 
> > >>> > > diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
> > >>> > > index d4aff29..561b987 100644
> > >>> > > --- a/hw/ioapic_common.c
> > >>> > > +++ b/hw/ioapic_common.c
> > >>> > > @@ -59,7 +59,7 @@ static int ioapic_dispatch_post_load(void
> > >>> > > *opaque, int version_id) 
> > >>> > >  static int ioapic_init_common(SysBusDevice *dev)
> > >>> > >  {
> > >>> > > -    IOAPICCommonState *s = FROM_SYSBUS(IOAPICCommonState, dev);
> > >>> > > +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
> > >>> > >      IOAPICCommonClass *info;
> > >>> > >      static int ioapic_no;
> > >>> > >  
> > >> > 
> > >> > 
> > >> > 
> > > sure, will do this on the next respin.
> > 
> > Actually no, why?  It's two different files.
> 
> because they do the same trivial thing,
> do you want some mechanic changes separated?

There is no point arguing,
Andreas applied the first patch to qom-cpu tree already,
and seems he is fine taking them as separate patches.
Andreas Färber April 10, 2013, 5:58 p.m. UTC | #7 
Am 05.04.2013 16:37, schrieb Igor Mammedov:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Thanks, applied to qom-cpu (dropping "target-i386"):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

> ---
>  hw/ioapic_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
> index d4aff29..561b987 100644
> --- a/hw/ioapic_common.c
> +++ b/hw/ioapic_common.c
> @@ -59,7 +59,7 @@ static int ioapic_dispatch_post_load(void *opaque, int version_id)
>  
>  static int ioapic_init_common(SysBusDevice *dev)
>  {
> -    IOAPICCommonState *s = FROM_SYSBUS(IOAPICCommonState, dev);
> +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
>      IOAPICCommonClass *info;
>      static int ioapic_no;
>
Andreas Färber April 10, 2013, 6:12 p.m. UTC | #8 
Am 10.04.2013 18:12, schrieb Igor Mammedov:
> On Wed, 10 Apr 2013 08:21:17 +0800
> li guang <lig.fnst@cn.fujitsu.com> wrote:
> 
>> 在 2013-04-09二的 13:36 +0200，Paolo Bonzini写道：
>>> Il 08/04/2013 13:32, Igor Mammedov ha scritto:
>>>>
>>>>>> This patch should be combined with [PATCH 07/22]
>>>>>>
>>>>>> 在 2013-04-05五的 16:37 +0200，Igor Mammedov写道：
>>>>>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>>>>>>> ---
>>>>>>>>  hw/ioapic_common.c | 2 +-
>>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
>>>>>>>> index d4aff29..561b987 100644
>>>>>>>> --- a/hw/ioapic_common.c
>>>>>>>> +++ b/hw/ioapic_common.c
>>>>>>>> @@ -59,7 +59,7 @@ static int ioapic_dispatch_post_load(void
>>>>>>>> *opaque, int version_id) 
>>>>>>>>  static int ioapic_init_common(SysBusDevice *dev)
>>>>>>>>  {
>>>>>>>> -    IOAPICCommonState *s = FROM_SYSBUS(IOAPICCommonState, dev);
>>>>>>>> +    IOAPICCommonState *s = IOAPIC_COMMON(dev);
>>>>>>>>      IOAPICCommonClass *info;
>>>>>>>>      static int ioapic_no;
>>>>>>>>  
>>>>>>
>>>>>>
>>>>>>
>>>> sure, will do this on the next respin.
>>>
>>> Actually no, why?  It's two different files.
>>
>> because they do the same trivial thing,
>> do you want some mechanic changes separated?
> 
> There is no point arguing,
> Andreas applied the first patch to qom-cpu tree already,
> and seems he is fine taking them as separate patches.

Yes, I had the second one in my testing queue, too. It is exactly the
way I'd expect it, modulo that it doesn't touch target-i386 directory it
mentions. That is assuming that there are no further FROM_SYSBUS() or
DO_UPCAST()s hidden elsewhere in the respective device, which I didn't
check yet.

Finding an appropriate subject for an equivalent change touching two
files is one aspect for separate patches, individually being able to
bisect problems another. If it were not two but a hundred one-line
patches, that were a different issue of course. But unfortunately there
are quite a lot of FROM_SYSBUS() users, most of which require more work
than this one here, so doing all in one "sysbus:" patch seems unrealistic.

BTW since these are SysBus devices, if someone has time to convert the
initfns to QOM realize as follow-ups, I would be happy to review.

Regards,
Andreas
diff mbox

Patch

diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
index d4aff29..561b987 100644
--- a/hw/ioapic_common.c
+++ b/hw/ioapic_common.c
@@ -59,7 +59,7 @@  static int ioapic_dispatch_post_load(void *opaque, int version_id)
 
 static int ioapic_init_common(SysBusDevice *dev)
 {
-    IOAPICCommonState *s = FROM_SYSBUS(IOAPICCommonState, dev);
+    IOAPICCommonState *s = IOAPIC_COMMON(dev);
     IOAPICCommonClass *info;
     static int ioapic_no;