Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french)

[John Snow <jsnow@redhat.com>]

On 6/22/20 5:42 AM, Markus Armbruster wrote:
> There are three ways to configure backends:
> 
> * -nic, -serial, -drive, ... (onboard devices)
> 
> * Set the property with -device, or, if you feel masochistic, with
>   -set device (pluggable devices)
> 
> * Set the property with -global (both)
> 
> The trouble is -global is terrible.
> 
> It gets applied in object_new(), which can't fail.  We treat failure
> to apply -global as fatal error, except when hot-plugging, where we
> treat it as warning *boggle*.  I'm not addressing that today.
> 
> Some code falls apart when you use both -global and the other way.
> 
> To make life more interesting, we gave -drive two roles: with
> interface type other than none, it's for configuring onboard devices,
> and with interface type none, it's for defining backends for use with
> -device and such.  Since we neglect to require interface type none for
> the latter, you can use one -drive in both roles.  This confuses the
> code about as much as you, dear reader, probably are by now.
> 
> Because this still isn't interesting enough, there's yet another way
> to configure backends, just for floppies: set the floppy controller's
> property.  Goes back to the time when floppy wasn't a separate device,
> and involves some Bad Magic.  Now -global can interact with itself!
> 
> Digging through all this took me an embarrassing amount of time.
> Hair, too.
> 
> My patches reject some the silliest uses outright, and deprecate some
> not so silly ones that have replacements.
> 
> Apply on top of my "[PATCH v2 00/58] qdev: Rework how we plug into the
> parent bus".
> 

Oof. Thank you for your work in fixing our darkest corners. I sincerely
appreciate it.

The qdev tree ordering problems don't cause any issues for migration, do
they?

(I see you already sent a PR, so whatever!)

> Enjoy!
> 
> v2:
> * Rebased; tests/qemu-iotests/172.out regenerated to resolve conflicts
> * PATCH 10-12: check_non_null() renamed to check_prop_still_unset()
>   [Philippe]
> 
> Markus Armbruster (16):
>   iotests/172: Include "info block" in test output
>   iotests/172: Cover empty filename and multiple use of drives
>   iotests/172: Cover -global floppy.drive=...
>   fdc: Reject clash between -drive if=floppy and -global isa-fdc
>   fdc: Open-code fdctrl_init_isa()
>   fdc: Deprecate configuring floppies with -global isa-fdc
>   docs/qdev-device-use.txt: Update section "Default Devices"
>   blockdev: Deprecate -drive with bogus interface type
>   qdev: Eliminate get_pointer(), set_pointer()
>   qdev: Improve netdev property override error a bit
>   qdev: Reject drive property override
>   qdev: Reject chardev property override
>   qdev: Make qdev_prop_set_drive() match the other helpers
>   arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp
>   sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error
>   sd/milkymist-memcard: Fix error API violation
> 
>  docs/qdev-device-use.txt            |  17 +-
>  docs/system/deprecated.rst          |  34 ++
>  include/hw/block/fdc.h              |   2 +-
>  include/hw/qdev-properties.h        |  18 +-
>  include/sysemu/blockdev.h           |   2 +
>  blockdev.c                          |  27 +-
>  hw/arm/aspeed.c                     |  16 +-
>  hw/arm/cubieboard.c                 |   2 +-
>  hw/arm/exynos4210.c                 |   2 +-
>  hw/arm/imx25_pdk.c                  |   2 +-
>  hw/arm/mcimx6ul-evk.c               |   2 +-
>  hw/arm/mcimx7d-sabre.c              |   2 +-
>  hw/arm/msf2-som.c                   |   4 +-
>  hw/arm/nseries.c                    |   4 +-
>  hw/arm/orangepi.c                   |   2 +-
>  hw/arm/raspi.c                      |   2 +-
>  hw/arm/sabrelite.c                  |   6 +-
>  hw/arm/vexpress.c                   |   3 +-
>  hw/arm/xilinx_zynq.c                |   7 +-
>  hw/arm/xlnx-versal-virt.c           |   2 +-
>  hw/arm/xlnx-zcu102.c                |  10 +-
>  hw/block/fdc.c                      |  82 ++--
>  hw/block/nand.c                     |   2 +-
>  hw/block/pflash_cfi01.c             |   6 +-
>  hw/block/pflash_cfi02.c             |   2 +-
>  hw/core/qdev-properties-system.c    | 151 ++++---
>  hw/core/qdev-properties.c           |  17 +
>  hw/i386/pc.c                        |   8 +-
>  hw/ide/qdev.c                       |   4 +-
>  hw/isa/isa-superio.c                |  18 +-
>  hw/m68k/q800.c                      |   3 +-
>  hw/microblaze/petalogix_ml605_mmu.c |   5 +-
>  hw/ppc/pnv.c                        |   3 +-
>  hw/ppc/spapr.c                      |   4 +-
>  hw/scsi/scsi-bus.c                  |   2 +-
>  hw/sd/milkymist-memcard.c           |   2 +-
>  hw/sd/pxa2xx_mmci.c                 |  15 +-
>  hw/sd/sd.c                          |   2 +-
>  hw/sd/ssi-sd.c                      |   3 +-
>  hw/sparc64/sun4u.c                  |   9 +-
>  hw/xtensa/xtfpga.c                  |   3 +-
>  softmmu/vl.c                        |   8 +
>  tests/qemu-iotests/172              |  27 +-
>  tests/qemu-iotests/172.out          | 656 +++++++++++++++++++++++++---
>  44 files changed, 928 insertions(+), 270 deletions(-)
>