Patchwork [PULL,00/45] Include reorganization + PCI patch queue

login
register
mail settings
Submitter Anthony Liguori
Date Dec. 19, 2012, 12:13 a.m.
Message ID <87r4mmnbl4.fsf@codemonkey.ws>
Download mbox | patch
Permalink /patch/207232/
State New
Headers show

Comments

Anthony Liguori - Dec. 19, 2012, 12:13 a.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> And how does it conflicts with Alex's ppc pull request? It is a fast
> forward from origin/master as of now (commit a8a826a, exec: refactor
> cpu_restore_state, 2012-12-04), and that includes Alex's commits...

It's not a conflict.  You both have problems with your pull requests
because I assume neither of you have libfdt installed which means a
bunch of ppc is not being built.

We really ought to pull in fdt as a submodule to prevent this type of
thing...

Paolo, you need to incorporate:


I can't do this through a merge commit because the branches being merged
aren't bisectable so please update and resend.  It's probably easiest to
do it all through Paolo's branch to make sure there are no conflicts
between the two branches.

Regards,

Anthony Liguori

>
> Paolo
>
>
>
> On Tue, Dec 18, 2012 at 10:11 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>> Il 18/12/2012 21:59, Anthony Liguori ha scritto:
>> > But this series breaks the build of make check.  It's not just one test
>> > case but almost every single one.  I think you must have eliminated an
>> > implicit include of qemu-common.h which makes PRId64 et al all
>> > undeclared.
>> >
>> > I started fixing this too but it became too much for a merge commit
>> > since it affects almost all tests.
>> >
>> > Can you fixup make check and send this series out?  I'll make sure to
>> > check tomorrow and merge your pull request if I see it tomorrow.
>> > Otherwise, let me know when you plan on sending it and I'll make sure to
>> > be available to merge it.
>>
>> Hmm, I must have pushed the wrong branch because I remember this failure.
>>
>> Paolo
>>
Anthony Liguori - Dec. 19, 2012, 2 a.m.
Anthony Liguori <aliguori@us.ibm.com> writes:

> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> And how does it conflicts with Alex's ppc pull request? It is a fast
>> forward from origin/master as of now (commit a8a826a, exec: refactor
>> cpu_restore_state, 2012-12-04), and that includes Alex's commits...
>
> It's not a conflict.  You both have problems with your pull requests
> because I assume neither of you have libfdt installed which means a
> bunch of ppc is not being built.
>
> We really ought to pull in fdt as a submodule to prevent this type of
> thing...

We the patches below, the changes pass all my tests so as soon as you
make these changes, it'll go in.

Regards,

Anthony Liguori

>
> Paolo, you need to incorporate:
>
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 5b16096..aa54fd8 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -18,7 +18,8 @@
>  #include "qemu-common.h"
>  #include "e500.h"
>  #include "e500-ccsr.h"
> -#include "net.h"
> +#include "net/net.h"
> +#include "qemu/config-file.h"
>  #include "hw/hw.h"
>  #include "hw/serial.h"
>  #include "hw/pci/pci.h"
>
> diff --git a/hw/spapr.c b/hw/spapr.c
> index 071cf41..fdd1eb6 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -49,6 +49,7 @@
>  
>  #include "exec/address-spaces.h"
>  #include "hw/usb.h"
> +#include "qemu/config-file.h"
>  
>  #include <libfdt.h>
>  
> Michael, you need to incorporate:
>
> diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
> index ea4134c..4deb02a 100644
> --- a/hw/ppc/e500plat.c
> +++ b/hw/ppc/e500plat.c
> @@ -14,7 +14,7 @@
>  #include "e500.h"
>  #include "../boards.h"
>  #include "sysemu/device_tree.h"
> -#include "hw/pci.h"
> +#include "hw/pci/pci.h"
>  
>  static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
>  {
>
> I can't do this through a merge commit because the branches being merged
> aren't bisectable so please update and resend.  It's probably easiest to
> do it all through Paolo's branch to make sure there are no conflicts
> between the two branches.
>
> Regards,
>
> Anthony Liguori
>
>>
>> Paolo
>>
>>
>>
>> On Tue, Dec 18, 2012 at 10:11 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>> Il 18/12/2012 21:59, Anthony Liguori ha scritto:
>>> > But this series breaks the build of make check.  It's not just one test
>>> > case but almost every single one.  I think you must have eliminated an
>>> > implicit include of qemu-common.h which makes PRId64 et al all
>>> > undeclared.
>>> >
>>> > I started fixing this too but it became too much for a merge commit
>>> > since it affects almost all tests.
>>> >
>>> > Can you fixup make check and send this series out?  I'll make sure to
>>> > check tomorrow and merge your pull request if I see it tomorrow.
>>> > Otherwise, let me know when you plan on sending it and I'll make sure to
>>> > be available to merge it.
>>>
>>> Hmm, I must have pushed the wrong branch because I remember this failure.
>>>
>>> Paolo
>>>
Alexander Graf - Dec. 19, 2012, 2:13 a.m.
On 19.12.2012, at 03:00, Anthony Liguori wrote:

> Anthony Liguori <aliguori@us.ibm.com> writes:
> 
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>>> And how does it conflicts with Alex's ppc pull request? It is a fast
>>> forward from origin/master as of now (commit a8a826a, exec: refactor
>>> cpu_restore_state, 2012-12-04), and that includes Alex's commits...
>> 
>> It's not a conflict.  You both have problems with your pull requests
>> because I assume neither of you have libfdt installed which means a
>> bunch of ppc is not being built.
>> 
>> We really ought to pull in fdt as a submodule to prevent this type of
>> thing...

Does making libfdt a submodule really change things? Developers would still have to init the submodule. They could just as well install the libfdt-devel packet from their distro of choice instead :).


Alex

> 
> We the patches below, the changes pass all my tests so as soon as you
> make these changes, it'll go in.
> 
> Regards,
> 
> Anthony Liguori
> 
>> 
>> Paolo, you need to incorporate:
>> 
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 5b16096..aa54fd8 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -18,7 +18,8 @@
>> #include "qemu-common.h"
>> #include "e500.h"
>> #include "e500-ccsr.h"
>> -#include "net.h"
>> +#include "net/net.h"
>> +#include "qemu/config-file.h"
>> #include "hw/hw.h"
>> #include "hw/serial.h"
>> #include "hw/pci/pci.h"
>> 
>> diff --git a/hw/spapr.c b/hw/spapr.c
>> index 071cf41..fdd1eb6 100644
>> --- a/hw/spapr.c
>> +++ b/hw/spapr.c
>> @@ -49,6 +49,7 @@
>> 
>> #include "exec/address-spaces.h"
>> #include "hw/usb.h"
>> +#include "qemu/config-file.h"
>> 
>> #include <libfdt.h>
>> 
>> Michael, you need to incorporate:
>> 
>> diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
>> index ea4134c..4deb02a 100644
>> --- a/hw/ppc/e500plat.c
>> +++ b/hw/ppc/e500plat.c
>> @@ -14,7 +14,7 @@
>> #include "e500.h"
>> #include "../boards.h"
>> #include "sysemu/device_tree.h"
>> -#include "hw/pci.h"
>> +#include "hw/pci/pci.h"
>> 
>> static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
>> {
>> 
>> I can't do this through a merge commit because the branches being merged
>> aren't bisectable so please update and resend.  It's probably easiest to
>> do it all through Paolo's branch to make sure there are no conflicts
>> between the two branches.
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>>> 
>>> Paolo
>>> 
>>> 
>>> 
>>> On Tue, Dec 18, 2012 at 10:11 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>> 
>>>> Il 18/12/2012 21:59, Anthony Liguori ha scritto:
>>>>> But this series breaks the build of make check.  It's not just one test
>>>>> case but almost every single one.  I think you must have eliminated an
>>>>> implicit include of qemu-common.h which makes PRId64 et al all
>>>>> undeclared.
>>>>> 
>>>>> I started fixing this too but it became too much for a merge commit
>>>>> since it affects almost all tests.
>>>>> 
>>>>> Can you fixup make check and send this series out?  I'll make sure to
>>>>> check tomorrow and merge your pull request if I see it tomorrow.
>>>>> Otherwise, let me know when you plan on sending it and I'll make sure to
>>>>> be available to merge it.
>>>> 
>>>> Hmm, I must have pushed the wrong branch because I remember this failure.
>>>> 
>>>> Paolo
>>>> 
>
David Gibson - Dec. 19, 2012, 2:18 a.m.
On Wed, Dec 19, 2012 at 03:13:10AM +0100, Alexander Graf wrote:
> 
> On 19.12.2012, at 03:00, Anthony Liguori wrote:
> 
> > Anthony Liguori <aliguori@us.ibm.com> writes:
> > 
> >> Paolo Bonzini <pbonzini@redhat.com> writes:
> >> 
> >>> And how does it conflicts with Alex's ppc pull request? It is a fast
> >>> forward from origin/master as of now (commit a8a826a, exec: refactor
> >>> cpu_restore_state, 2012-12-04), and that includes Alex's commits...
> >> 
> >> It's not a conflict.  You both have problems with your pull requests
> >> because I assume neither of you have libfdt installed which means a
> >> bunch of ppc is not being built.
> >> 
> >> We really ought to pull in fdt as a submodule to prevent this type of
> >> thing...
> 
> Does making libfdt a submodule really change things? Developers
> would still have to init the submodule. They could just as well
> install the libfdt-devel packet from their distro of choice instead
> :).

Presumably adding the submodule would go along with changes to
configure to make libfdt always selected.
Michael S. Tsirkin - Dec. 19, 2012, 6:14 a.m.
On Tue, Dec 18, 2012 at 06:13:11PM -0600, Anthony Liguori wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > And how does it conflicts with Alex's ppc pull request? It is a fast
> > forward from origin/master as of now (commit a8a826a, exec: refactor
> > cpu_restore_state, 2012-12-04), and that includes Alex's commits...
> 
> It's not a conflict.  You both have problems with your pull requests
> because I assume neither of you have libfdt installed which means a
> bunch of ppc is not being built.
> 
> We really ought to pull in fdt as a submodule to prevent this type of
> thing...

Same applies to any library.  How about an in-tree script
with configure flags that we want tested instead?
Then it would fail instead of disabling silently.
Paolo Bonzini - Dec. 19, 2012, 8:35 a.m.
Il 19/12/2012 03:13, Alexander Graf ha scritto:
> Does making libfdt a submodule really change things? Developers would
> still have to init the submodule. They could just as well install the
> libfdt-devel packet from their distro of choice instead :).

RHEL does not have it unfortunately.  I requested it to be added to EPEL
so that us Red Hatters can do more comprehensive testing even when
running on the "enterprise" distro. :)

Paolo
Michael S. Tsirkin - Dec. 19, 2012, 7:45 p.m.
On Tue, Dec 18, 2012 at 08:00:58PM -0600, Anthony Liguori wrote:
> Anthony Liguori <aliguori@us.ibm.com> writes:
> 
> > Paolo Bonzini <pbonzini@redhat.com> writes:
> >
> >> And how does it conflicts with Alex's ppc pull request? It is a fast
> >> forward from origin/master as of now (commit a8a826a, exec: refactor
> >> cpu_restore_state, 2012-12-04), and that includes Alex's commits...
> >
> > It's not a conflict.  You both have problems with your pull requests
> > because I assume neither of you have libfdt installed which means a
> > bunch of ppc is not being built.
> >
> > We really ought to pull in fdt as a submodule to prevent this type of
> > thing...
> 
> We the patches below, the changes pass all my tests so as soon as you
> make these changes, it'll go in.
> 
> Regards,
> 
> Anthony Liguori

I do have fdt actually, but build still passed for me.
And I touched hw/ppc/e500plat.c and checked that it does
get compiled.
In my tree hw/ppc/e500plat.c did not include hw/pci.c
so it's something to do with the merge.

Maybe after all you could merge the pci tree?
If Paolo merges it too it will be trivial to
merge his ...

> >
> > Paolo, you need to incorporate:
> >
> > diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> > index 5b16096..aa54fd8 100644
> > --- a/hw/ppc/e500.c
> > +++ b/hw/ppc/e500.c
> > @@ -18,7 +18,8 @@
> >  #include "qemu-common.h"
> >  #include "e500.h"
> >  #include "e500-ccsr.h"
> > -#include "net.h"
> > +#include "net/net.h"
> > +#include "qemu/config-file.h"
> >  #include "hw/hw.h"
> >  #include "hw/serial.h"
> >  #include "hw/pci/pci.h"
> >
> > diff --git a/hw/spapr.c b/hw/spapr.c
> > index 071cf41..fdd1eb6 100644
> > --- a/hw/spapr.c
> > +++ b/hw/spapr.c
> > @@ -49,6 +49,7 @@
> >  
> >  #include "exec/address-spaces.h"
> >  #include "hw/usb.h"
> > +#include "qemu/config-file.h"
> >  
> >  #include <libfdt.h>
> >  
> > Michael, you need to incorporate:
> >
> > diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
> > index ea4134c..4deb02a 100644
> > --- a/hw/ppc/e500plat.c
> > +++ b/hw/ppc/e500plat.c
> > @@ -14,7 +14,7 @@
> >  #include "e500.h"
> >  #include "../boards.h"
> >  #include "sysemu/device_tree.h"
> > -#include "hw/pci.h"
> > +#include "hw/pci/pci.h"
> >  
> >  static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
> >  {
> >
> > I can't do this through a merge commit because the branches being merged
> > aren't bisectable so please update and resend.  It's probably easiest to
> > do it all through Paolo's branch to make sure there are no conflicts
> > between the two branches.
> >
> > Regards,
> >
> > Anthony Liguori
> >
> >>
> >> Paolo
> >>
> >>
> >>
> >> On Tue, Dec 18, 2012 at 10:11 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >>
> >>> Il 18/12/2012 21:59, Anthony Liguori ha scritto:
> >>> > But this series breaks the build of make check.  It's not just one test
> >>> > case but almost every single one.  I think you must have eliminated an
> >>> > implicit include of qemu-common.h which makes PRId64 et al all
> >>> > undeclared.
> >>> >
> >>> > I started fixing this too but it became too much for a merge commit
> >>> > since it affects almost all tests.
> >>> >
> >>> > Can you fixup make check and send this series out?  I'll make sure to
> >>> > check tomorrow and merge your pull request if I see it tomorrow.
> >>> > Otherwise, let me know when you plan on sending it and I'll make sure to
> >>> > be available to merge it.
> >>>
> >>> Hmm, I must have pushed the wrong branch because I remember this failure.
> >>>
> >>> Paolo
> >>>
Blue Swirl - Dec. 19, 2012, 8:02 p.m.
On Wed, Dec 19, 2012 at 8:35 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 19/12/2012 03:13, Alexander Graf ha scritto:
>> Does making libfdt a submodule really change things? Developers would
>> still have to init the submodule. They could just as well install the
>> libfdt-devel packet from their distro of choice instead :).
>
> RHEL does not have it unfortunately.  I requested it to be added to EPEL
> so that us Red Hatters can do more comprehensive testing even when
> running on the "enterprise" distro. :)

It's also not available on Debian, I don't think the BSDs or Windows
have it either.

>
> Paolo
>

Patch

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 5b16096..aa54fd8 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -18,7 +18,8 @@ 
 #include "qemu-common.h"
 #include "e500.h"
 #include "e500-ccsr.h"
-#include "net.h"
+#include "net/net.h"
+#include "qemu/config-file.h"
 #include "hw/hw.h"
 #include "hw/serial.h"
 #include "hw/pci/pci.h"

diff --git a/hw/spapr.c b/hw/spapr.c
index 071cf41..fdd1eb6 100644
--- a/hw/spapr.c
+++ b/hw/spapr.c
@@ -49,6 +49,7 @@ 
 
 #include "exec/address-spaces.h"
 #include "hw/usb.h"
+#include "qemu/config-file.h"
 
 #include <libfdt.h>
 
Michael, you need to incorporate:

diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
index ea4134c..4deb02a 100644
--- a/hw/ppc/e500plat.c
+++ b/hw/ppc/e500plat.c
@@ -14,7 +14,7 @@ 
 #include "e500.h"
 #include "../boards.h"
 #include "sysemu/device_tree.h"
-#include "hw/pci.h"
+#include "hw/pci/pci.h"
 
 static void e500plat_fixup_devtree(PPCE500Params *params, void *fdt)
 {