[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 00/16] Crazy shit around -global (pardon my french)
From: |
John Snow |
Subject: |
Re: [PATCH 00/16] Crazy shit around -global (pardon my french) |
Date: |
Wed, 10 Jun 2020 10:21:24 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 6/5/20 10:56 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".
>
> Enjoy!
>
> Based-on: <20200529134523.8477-1-armbru@redhat.com>
>
> 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(-)
>
I'll be honest that I'm a little pre-occupied and possibly unable to
review the fdc related changes in-depth. I generally trust your
judgment, and will try to give it a quick scan.
You may treat any further silence as an ACK. Any breakage due to this
policy is therefore assumed to be the liability of the maintainer.
--js
- Re: [PATCH 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp, (continued)
- [PATCH 09/16] qdev: Eliminate get_pointer(), set_pointer(), Markus Armbruster, 2020/06/05
- [PATCH 02/16] iotests/172: Cover empty filename and multiple use of drives, Markus Armbruster, 2020/06/05
- [PATCH 16/16] sd/milkymist-memcard: Fix error API violation, Markus Armbruster, 2020/06/05
- [PATCH 12/16] qdev: Reject chardev property override, Markus Armbruster, 2020/06/05
- [PATCH 04/16] fdc: Reject clash between -drive if=floppy and -global isa-fdc, Markus Armbruster, 2020/06/05
- Re: [PATCH 00/16] Crazy shit around -global (pardon my french),
John Snow <=