The following changes since commit 14639717bf379480e937716fcaf1e72b47fd4c5f:

  Merge tag 'pull-trivial-patches' of https://gitlab.com/mjt0k/qemu into staging (2024-01-31 19:53:45 +0000)

are available in the Git repository at:

  git://git.infradead.org/users/dwmw2/qemu.git tags/pull-nic-config.for-upstream-20240201

for you to fetch changes up to 5382a24c9b0be4391ea91b020bb72ad15c05cc88:

  net: make nb_nics and nd_table[] static in net/net.c (2024-02-01 16:21:52 +0000)

----------------------------------------------------------------
Rework matching of network devices to -nic options

----------------------------------------------------------------
David Woodhouse (47):
      net: add qemu_{configure,create}_nic_device(), qemu_find_nic_info()
      net: report list of available models according to platform
      net: add qemu_create_nic_bus_devices()
      hw/pci: add pci_init_nic_devices(), pci_init_nic_in_slot()
      hw/i386/pc: use qemu_get_nic_info() and pci_init_nic_devices()
      hw/xen: use qemu_create_nic_bus_devices() to instantiate Xen NICs
      hw/alpha/dp264: use pci_init_nic_devices()
      hw/arm/sbsa-ref: use pci_init_nic_devices()
      hw/arm/virt: use pci_init_nic_devices()
      hw/hppa: use pci_init_nic_devices()
      hw/loongarch: use pci_init_nic_devices()
      hw/mips/fuloong2e: use pci_init_nic_devices()
      hw/mips/malta: use pci_init_nic_devices()
      hw/mips/loongson3_virt: use pci_init_nic_devices()
      hw/ppc/prep: use pci_init_nic_devices()
      hw/ppc/spapr: use qemu_get_nic_info() and pci_init_nic_devices()
      hw/ppc: use pci_init_nic_devices()
      hw/sh4/r2d: use pci_init_nic_devices()
      hw/sparc64/sun4u: use pci_init_nic_devices()
      hw/xtensa/virt: use pci_init_nic_devices()
      hw/arm/allwinner: use qemu_configure_nic_device()
      hw/arm/aspeed: use qemu_configure_nic_device()
      hw/arm/exynos4: use qemu_create_nic_device()
      hw/arm/fsl: use qemu_configure_nic_device()
      hw/net/smc91c111: use qemu_configure_nic_device()
      hw/net/lan9118: use qemu_configure_nic_device()
      hw/arm/highbank: use qemu_create_nic_device()
      hw/arm/npcm7xx: use qemu_configure_nic_device, allow emc0/emc1 as aliases
      hw/arm/stellaris: use qemu_find_nic_info()
      hw/arm: use qemu_configure_nic_device()
      hw/net/etraxfs-eth: use qemu_configure_nic_device()
      hw/m68k/mcf5208: use qemu_create_nic_device()
      hw/m68k/q800: use qemu_find_nic_info()
      hw/microblaze: use qemu_configure_nic_device()
      hw/mips/mipssim: use qemu_create_nic_device()
      hw/mips/jazz: use qemu_find_nic_info()
      hw/net/lasi_i82596: Re-enable build
      hw/net/lasi_i82596: use qemu_create_nic_device()
      hw/openrisc/openrisc_sim: use qemu_create_nic_device()
      hw/riscv: use qemu_configure_nic_device()
      hw/s390x/s390-virtio-ccw: use qemu_create_nic_device()
      hw/sparc/sun4m: use qemu_find_nic_info()
      hw/xtensa/xtfpga: use qemu_create_nic_device()
      net: remove qemu_check_nic_model()
      hw/pci: remove pci_nic_init_nofail()
      net: remove qemu_show_nic_models(), qemu_find_nic_model()
      net: make nb_nics and nd_table[] static in net/net.c

 hw/alpha/dp264.c                         |   4 +-
 hw/arm/allwinner-a10.c                   |   6 +-
 hw/arm/allwinner-h3.c                    |   6 +-
 hw/arm/allwinner-r40.c                   |  27 +---
 hw/arm/aspeed.c                          |   9 +-
 hw/arm/exynos4_boards.c                  |   6 +-
 hw/arm/fsl-imx25.c                       |   2 +-
 hw/arm/fsl-imx6.c                        |   2 +-
 hw/arm/fsl-imx6ul.c                      |   2 +-
 hw/arm/fsl-imx7.c                        |   2 +-
 hw/arm/gumstix.c                         |   6 +-
 hw/arm/highbank.c                        |  12 +-
 hw/arm/integratorcp.c                    |   5 +-
 hw/arm/kzm.c                             |   4 +-
 hw/arm/mainstone.c                       |   3 +-
 hw/arm/mps2-tz.c                         |   8 +-
 hw/arm/mps2.c                            |   2 +-
 hw/arm/msf2-soc.c                        |   6 +-
 hw/arm/musicpal.c                        |   3 +-
 hw/arm/npcm7xx.c                         |  16 +-
 hw/arm/realview.c                        |  25 ++-
 hw/arm/sbsa-ref.c                        |   4 +-
 hw/arm/stellaris.c                       |  30 +++-
 hw/arm/versatilepb.c                     |  15 +-
 hw/arm/vexpress.c                        |   4 +-
 hw/arm/virt.c                            |   4 +-
 hw/arm/xilinx_zynq.c                     |  11 +-
 hw/arm/xlnx-versal.c                     |   7 +-
 hw/arm/xlnx-zynqmp.c                     |   8 +-
 hw/cris/axis_dev88.c                     |   9 +-
 hw/hppa/machine.c                        |  14 +-
 hw/i386/pc.c                             |  38 +++--
 hw/i386/pc_piix.c                        |   2 +-
 hw/i386/pc_q35.c                         |   2 +-
 hw/loongarch/virt.c                      |   4 +-
 hw/m68k/mcf5208.c                        |  19 +--
 hw/m68k/q800.c                           |  29 ++--
 hw/microblaze/petalogix_ml605_mmu.c      |   3 +-
 hw/microblaze/petalogix_s3adsp1800_mmu.c |   3 +-
 hw/mips/fuloong2e.c                      |  16 +-
 hw/mips/jazz.c                           |  15 +-
 hw/mips/loongson3_virt.c                 |   4 +-
 hw/mips/malta.c                          |  15 +-
 hw/mips/mipssim.c                        |  13 +-
 hw/net/etraxfs_eth.c                     |   5 +-
 hw/net/lan9118.c                         |   5 +-
 hw/net/lasi_i82596.c                     |  13 +-
 hw/net/meson.build                       |   2 +-
 hw/net/smc91c111.c                       |   5 +-
 hw/openrisc/openrisc_sim.c               |  18 +--
 hw/pci/pci.c                             |  73 +++------
 hw/ppc/e500.c                            |   4 +-
 hw/ppc/mac_newworld.c                    |   4 +-
 hw/ppc/mac_oldworld.c                    |   4 +-
 hw/ppc/ppc440_bamboo.c                   |  14 +-
 hw/ppc/prep.c                            |   8 +-
 hw/ppc/spapr.c                           |  18 +--
 hw/riscv/microchip_pfsoc.c               |  14 +-
 hw/riscv/sifive_u.c                      |   7 +-
 hw/s390x/s390-virtio-ccw.c               |  11 +-
 hw/sh4/r2d.c                             |   6 +-
 hw/sparc/sun4m.c                         |  20 ++-
 hw/sparc64/sun4u.c                       |  27 +---
 hw/xen/xen-bus.c                         |   6 +-
 hw/xen/xen_devconfig.c                   |  25 ---
 hw/xenpv/xen_machine_pv.c                |   9 --
 hw/xtensa/virt.c                         |   4 +-
 hw/xtensa/xtfpga.c                       |  13 +-
 include/hw/cris/etraxfs.h                |   2 +-
 include/hw/i386/pc.h                     |   4 +-
 include/hw/net/lan9118.h                 |   2 +-
 include/hw/net/lasi_82596.h              |   4 +-
 include/hw/net/ne2000-isa.h              |   2 -
 include/hw/net/smc91c111.h               |   2 +-
 include/hw/pci/pci.h                     |   7 +-
 include/hw/xen/xen-bus.h                 |   2 +-
 include/hw/xen/xen-legacy-backend.h      |   1 -
 include/net/net.h                        |  70 ++++++++-
 net/net.c                                | 253 +++++++++++++++++++++++++------
 system/globals.c                         |   2 -
 tests/qtest/npcm7xx_emc-test.c           |  18 +--
 81 files changed, 547 insertions(+), 562 deletions(-)

On Thu, 1 Feb 2024 at 16:48, David Woodhouse <dwmw2@infradead.org> wrote:
>
> The following changes since commit 14639717bf379480e937716fcaf1e72b47fd4c5f:
>
>   Merge tag 'pull-trivial-patches' of https://gitlab.com/mjt0k/qemu into staging (2024-01-31 19:53:45 +0000)
>
> are available in the Git repository at:
>
>   git://git.infradead.org/users/dwmw2/qemu.git tags/pull-nic-config.for-upstream-20240201
>
> for you to fetch changes up to 5382a24c9b0be4391ea91b020bb72ad15c05cc88:
>
>   net: make nb_nics and nd_table[] static in net/net.c (2024-02-01 16:21:52 +0000)
>
> ----------------------------------------------------------------
> Rework matching of network devices to -nic options

This fails "make check' because some of the qom-test and
test-hmp checks fail when the QEMU binary segfaults.

https://gitlab.com/qemu-project/qemu/-/jobs/6084552256
https://gitlab.com/qemu-project/qemu/-/jobs/6084044180

Reproduces on an x86-64 host if you do a build and 'make check'
for all target archs.

Generally this kind of segfault is because some machine type
segfaults on startup. For instance:

$ ./build/all/qemu-system-hppa -M C3700
Segmentation fault (core dumped)

(It's possible that's the only machine type that has a
problem; I haven't tried to exhaustively check.)

-- PMM

On Fri, 2024-02-02 at 15:32 +0000, Peter Maydell wrote:
> 
> $ ./build/all/qemu-system-hppa -M C3700
> Segmentation fault (core dumped)

Ah, got it. Some HPPA machine types don't have a lasi_dev.


--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -362,9 +362,11 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
     }
 
     /* Network setup. */
-    lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA),
-                    qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA),
-                    enable_lasi_lan());
+    if (lasi_dev) {
+        lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA),
+                        qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA),
+                        enable_lasi_lan());
+    }
 
     pci_init_nic_devices(pci_bus, mc->default_nic);
 
New pipeline running (FWIW) at
https://gitlab.com/dwmw2/qemu/-/pipelines/1162635873

What is the next step? Post the full series as a v5, or perhaps just
the single fixed patch which is now at
https://git.infradead.org/?p=users/dwmw2/qemu.git;a=commitdiff;h=2c20b4ee96db

... and then another pull request? The docs are fairly clear that pull
requests can't have even minor changes that haven't been posted
separately... and I guess the above incremental doesn't count?

Sorry for the noise.

On Fri, 2 Feb 2024 at 16:01, David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Fri, 2024-02-02 at 15:32 +0000, Peter Maydell wrote:
> >
> > $ ./build/all/qemu-system-hppa -M C3700
> > Segmentation fault (core dumped)
>
> Ah, got it. Some HPPA machine types don't have a lasi_dev.
>
>
> --- a/hw/hppa/machine.c
> +++ b/hw/hppa/machine.c
> @@ -362,9 +362,11 @@ static void machine_HP_common_init_tail(MachineState *machine, PCIBus *pci_bus,
>      }
>
>      /* Network setup. */
> -    lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA),
> -                    qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA),
> -                    enable_lasi_lan());
> +    if (lasi_dev) {
> +        lasi_82596_init(addr_space, translate(NULL, LASI_LAN_HPA),
> +                        qdev_get_gpio_in(lasi_dev, LASI_IRQ_LAN_HPA),
> +                        enable_lasi_lan());
> +    }
>
>      pci_init_nic_devices(pci_bus, mc->default_nic);
>
> New pipeline running (FWIW) at
> https://gitlab.com/dwmw2/qemu/-/pipelines/1162635873
>
> What is the next step? Post the full series as a v5, or perhaps just
> the single fixed patch which is now at
> https://git.infradead.org/?p=users/dwmw2/qemu.git;a=commitdiff;h=2c20b4ee96db
>
> ... and then another pull request?

If that diff above is the only change, then:
 * roll a new pullrequest with the fix squashed into the appropriate patch
 * have the subject marker be "PULL v2"
 * you can send just the cover-letter and the one patch that has changed,
   you don't need to resend the entire series (though it's not a big
   deal if you do send the whole set of mails again)

> The docs are fairly clear that pull
> requests can't have even minor changes that haven't been posted
> separately... and I guess the above incremental doesn't count?

Which bit of the docs is that? It's not our actual practice,
so we should really fix the wording. The principle is "don't
stick code into pullreqs that hasn't been through the review
process", but in practice especially for submaintainers who
know the system it's not uncommon to say "I'm going to
squash change X in and take this" or similar rather than
forcing submitters to do another round of sending out patches.
There should be *something* on the list to say this change was
put in, but eg the exchange in this email thread is fine for that.

thanks
-- PMM

On Fri, 2024-02-02 at 16:15 +0000, Peter Maydell wrote:
> On Fri, 2 Feb 2024 at 16:01, David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > What is the next step? Post the full series as a v5, or perhaps just
> > the single fixed patch which is now at
> > https://git.infradead.org/?p=users/dwmw2/qemu.git;a=commitdiff;h=2c20b4ee96db
> > 
> > ... and then another pull request?
> 
> If that diff above is the only change, then:
>  * roll a new pullrequest with the fix squashed into the appropriate patch
>  * have the subject marker be "PULL v2"
>  * you can send just the cover-letter and the one patch that has changed,
>    you don't need to resend the entire series (though it's not a big
>    deal if you do send the whole set of mails again)

Done, thank you.

> > The docs are fairly clear that pull
> > requests can't have even minor changes that haven't been posted
> > separately... and I guess the above incremental doesn't count?
> 
> Which bit of the docs is that? It's not our actual practice,
> so we should really fix the wording. The principle is "don't
> stick code into pullreqs that hasn't been through the review
> process", but in practice especially for submaintainers who
> know the system it's not uncommon to say "I'm going to
> squash change X in and take this" or similar rather than
> forcing submitters to do another round of sending out patches.
> There should be *something* on the list to say this change was
> put in, but eg the exchange in this email thread is fine for that.

I think I was looking at
https://www.qemu.org/docs/master/devel/submitting-a-pull-request.html
where it says "In particular if you’ve corrected issues in one round of
code review, you need to send your fixed patch series as normal to the
list; you can’t put it in a pull request until it’s gone through.
(Extremely trivial fixes may be OK to just fix in passing, but if in
doubt err on the side of not.)"

I think the doc is actually fine and I was just reading it too
conservatively. I have been making a conscious effort to pay attention,
follow the process and fit in, rather than just turning up and doing
whatever comes naturally from decades of working on open source. (I do
not *promise* that will last.)

On Fri, 2024-02-02 at 15:32 +0000, Peter Maydell wrote:
> 
> This fails "make check' because some of the qom-test and
> test-hmp checks fail when the QEMU binary segfaults.
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/6084552256
> https://gitlab.com/qemu-project/qemu/-/jobs/6084044180

Thanks.  Any idea why that didn't show up in my own pipeline?
https://gitlab.com/dwmw2/qemu/-/pipelines/1160949234

(The only difference in that tree is a last minute Reviewed-by:)
 
> Reproduces on an x86-64 host if you do a build and 'make check'
> for all target archs.
> 
> Generally this kind of segfault is because some machine type
> segfaults on startup. For instance:
> 
> $ ./build/all/qemu-system-hppa -M C3700
> Segmentation fault (core dumped)
> 
> (It's possible that's the only machine type that has a
> problem; I haven't tried to exhaustively check.)

Sorry about that. I'll go improve my test coverage...

On Fri, 2 Feb 2024 at 15:36, David Woodhouse <dwmw2@infradead.org> wrote:
>
> On Fri, 2024-02-02 at 15:32 +0000, Peter Maydell wrote:
> >
> > This fails "make check' because some of the qom-test and
> > test-hmp checks fail when the QEMU binary segfaults.
> >
> > https://gitlab.com/qemu-project/qemu/-/jobs/6084552256
> > https://gitlab.com/qemu-project/qemu/-/jobs/6084044180
>
> Thanks.  Any idea why that didn't show up in my own pipeline?
> https://gitlab.com/dwmw2/qemu/-/pipelines/1160949234

I think because the failing runners are the aarch64 and
s390 host ones, which we don't let run for anything
except real merge-pullreq test runs because they're
limited resource. I guess that perhaps we have at some point
said "we don't need to run all the guest architectures
on all jobs" and not noticed that this leaves the
coverage for the submaintainer only-uses-the-public-runners
CI testing with gaps.

CCing Alex and Thomas for possible suggestions.

thanks
-- PMM

On 02/02/2024 16.40, Peter Maydell wrote:
> On Fri, 2 Feb 2024 at 15:36, David Woodhouse <dwmw2@infradead.org> wrote:
>>
>> On Fri, 2024-02-02 at 15:32 +0000, Peter Maydell wrote:
>>>
>>> This fails "make check' because some of the qom-test and
>>> test-hmp checks fail when the QEMU binary segfaults.
>>>
>>> https://gitlab.com/qemu-project/qemu/-/jobs/6084552256
>>> https://gitlab.com/qemu-project/qemu/-/jobs/6084044180
>>
>> Thanks.  Any idea why that didn't show up in my own pipeline?
>> https://gitlab.com/dwmw2/qemu/-/pipelines/1160949234
> 
> I think because the failing runners are the aarch64 and
> s390 host ones, which we don't let run for anything
> except real merge-pullreq test runs because they're
> limited resource. I guess that perhaps we have at some point
> said "we don't need to run all the guest architectures
> on all jobs"

It's rather "we cannot run all the guest architectures on all jobs due to 
time constraints"

> and not noticed that this leaves the
> coverage for the submaintainer only-uses-the-public-runners
> CI testing with gaps.
>
> CCing Alex and Thomas for possible suggestions.

Well, not everybody has access to non-x86 machines, so there's not too much 
we can do about this, can we? The only possibility is: We still have support 
for travis-CI, so if you've got a github account, you can test aarch64, 
s390x and ppc64le there before sending a pull request. The .travis-ci.yml 
file needs some love, though, one of the jobs is currently timing out ... 
it's on my TODO list since weeks, but I just didn't find enough spare time 
to fix it properly yet.

  Thomas

On 05/02/2024 07.56, Thomas Huth wrote:
> On 02/02/2024 16.40, Peter Maydell wrote:
>> On Fri, 2 Feb 2024 at 15:36, David Woodhouse <dwmw2@infradead.org> wrote:
>>>
>>> On Fri, 2024-02-02 at 15:32 +0000, Peter Maydell wrote:
>>>>
>>>> This fails "make check' because some of the qom-test and
>>>> test-hmp checks fail when the QEMU binary segfaults.
>>>>
>>>> https://gitlab.com/qemu-project/qemu/-/jobs/6084552256
>>>> https://gitlab.com/qemu-project/qemu/-/jobs/6084044180
>>>
>>> Thanks.  Any idea why that didn't show up in my own pipeline?
>>> https://gitlab.com/dwmw2/qemu/-/pipelines/1160949234
>>
>> I think because the failing runners are the aarch64 and
>> s390 host ones, which we don't let run for anything
>> except real merge-pullreq test runs because they're
>> limited resource. I guess that perhaps we have at some point
>> said "we don't need to run all the guest architectures
>> on all jobs"
> 
> It's rather "we cannot run all the guest architectures on all jobs due to 
> time constraints"

Ah, wait, but we should still run at least "make check" for each target 
architecture... so there's indeed something that went wrong recently:

commit 78ebc00b06813 ("gitlab: shuffle some targets and reduce avocado 
noise") removed the hppa-softmmu target from the ubuntu job, without making 
sure that it gets tested somewhere else.

Alex, why did you remove it? It's now missing from all check-system-* jobs...

  Thomas

On Mon, 5 Feb 2024 at 10:11, Thomas Huth <thuth@redhat.com> wrote:
>
> On 05/02/2024 07.56, Thomas Huth wrote:
> > On 02/02/2024 16.40, Peter Maydell wrote:
> >> On Fri, 2 Feb 2024 at 15:36, David Woodhouse <dwmw2@infradead.org> wrote:
> >>>
> >>> On Fri, 2024-02-02 at 15:32 +0000, Peter Maydell wrote:
> >>>>
> >>>> This fails "make check' because some of the qom-test and
> >>>> test-hmp checks fail when the QEMU binary segfaults.
> >>>>
> >>>> https://gitlab.com/qemu-project/qemu/-/jobs/6084552256
> >>>> https://gitlab.com/qemu-project/qemu/-/jobs/6084044180
> >>>
> >>> Thanks.  Any idea why that didn't show up in my own pipeline?
> >>> https://gitlab.com/dwmw2/qemu/-/pipelines/1160949234
> >>
> >> I think because the failing runners are the aarch64 and
> >> s390 host ones, which we don't let run for anything
> >> except real merge-pullreq test runs because they're
> >> limited resource. I guess that perhaps we have at some point
> >> said "we don't need to run all the guest architectures
> >> on all jobs"
> >
> > It's rather "we cannot run all the guest architectures on all jobs due to
> > time constraints"
>
> Ah, wait, but we should still run at least "make check" for each target
> architecture...

Yes, this is what I mean -- it's OK to have the target
architecture checks split up between different CI jobs
as long as between the jobs we cover them all, but it's
not so good to be relying on the non-x86 host jobs to
provide part of the coverage.

-- PMM