From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: kwolf@redhat.com, berrange@redhat.com, ehabkost@redhat.com,
qemu-block@nongnu.org, qemu-devel@nongnu.org, mreitz@redhat.com,
pbonzini@redhat.com, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH v2 00/16] Crazy shit around -global (pardon my french)
Date: Mon, 29 Jun 2020 09:39:22 +0100 [thread overview]
Message-ID: <20200629083922.GC2908@work-vm> (raw)
In-Reply-To: <874kqwr953.fsf@dusky.pond.sub.org>
* Markus Armbruster (armbru@redhat.com) wrote:
> Cc: David for insurance against me spewing nonsense about migration.
>
> John Snow <jsnow@redhat.com> writes:
>
> > On 6/25/20 12:45 AM, Markus Armbruster wrote:
> >> John Snow <jsnow@redhat.com> writes:
> >>
> >>> 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?
> >>
> >> This series should only change device configuration, not device state or
> >> its encoding in the migration stream.
> >>
> >> I'm not sure what you mean by "qdev tree ordering problems". Ist it
> >> commit e8c9e65816 'qom: Make "info qom-tree" show children sorted'?
> >>
> >>> (I see you already sent a PR, so whatever!)
> >>
> >> A question that might avoid a later migration debugging session is
> >> *never* "whatever"!
> >>
> >
> > I thought I had read that one of these patches changes the order in
> > which devices get instantiated, which I thought might change their QOM
> > paths. Which I thought *might* have some ramifications for migration,
> > but wasn't sure.
>
> Device instantiation order changes should not break migration.
They shouldn't; although I only narrowly stopped a new device from
making a mistake that would have made it dependent.
Of course you do have to explicitly state PCI/USB slot IDs otherwise the
allocation of those is order dependent.
> The order in which devices appear in the migration stream should not
> matter.
Order in the stream is a separate issue; we have ways to enforce that;
for example you want the interrupt controller to arrive before a device
that will raise an interrupt.
Dave
Dave
> > If it's just showing the same path outputs *sorted*, then there's no
> > problem.
> >
> > Likely misread.
> >
> > --js
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
prev parent reply other threads:[~2020-06-29 8:41 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-22 9:42 [PATCH v2 00/16] Crazy shit around -global (pardon my french) Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 01/16] iotests/172: Include "info block" in test output Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 02/16] iotests/172: Cover empty filename and multiple use of drives Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 03/16] iotests/172: Cover -global floppy.drive= Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 04/16] fdc: Reject clash between -drive if=floppy and -global isa-fdc Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 05/16] fdc: Open-code fdctrl_init_isa() Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 06/16] fdc: Deprecate configuring floppies with -global isa-fdc Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 07/16] docs/qdev-device-use.txt: Update section "Default Devices" Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 08/16] blockdev: Deprecate -drive with bogus interface type Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 09/16] qdev: Eliminate get_pointer(), set_pointer() Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 10/16] qdev: Improve netdev property override error a bit Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 11/16] qdev: Reject drive property override Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 12/16] qdev: Reject chardev " Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 13/16] qdev: Make qdev_prop_set_drive() match the other helpers Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 14/16] arm/aspeed: Drop aspeed_board_init_flashes() parameter @errp Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 15/16] sd/pxa2xx_mmci: Don't crash on pxa2xx_mmci_init() error Markus Armbruster
2020-06-22 9:42 ` Markus Armbruster
2020-06-22 9:42 ` [PATCH v2 16/16] sd/milkymist-memcard: Fix error API violation Markus Armbruster
2020-06-22 9:56 ` Philippe Mathieu-Daudé
2020-06-22 10:07 ` [PATCH v2 00/16] Crazy shit around -global (pardon my french) no-reply
2020-06-24 15:40 ` John Snow
2020-06-25 4:45 ` Markus Armbruster
2020-06-26 15:11 ` John Snow
2020-06-27 12:22 ` Markus Armbruster
2020-06-29 8:39 ` Dr. David Alan Gilbert [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200629083922.GC2908@work-vm \
--to=dgilbert@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=ehabkost@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.