Peter Maydell <peter.mayd...@linaro.org> writes: > On Fri, 30 Nov 2018 at 07:40, Markus Armbruster <arm...@redhat.com> wrote: >> Peter Maydell <peter.mayd...@linaro.org> writes: >> > Add an assert somewhere and catch it with the usual >> > "instantiate everything" qtest? > >> The troublemaker is (3), where we may end up with an overridden >> realize-like method and an non-matching, un-overridden unrealize-like >> method. Got any smart ideas? > > Mmm, I see the difficulty. I suppose in theory we could have > a custom linter like clang-tidy and check that anything that > sets a realize pointer also sets an unrealize, but that's > pie-in-the-sky territory at the moment. > > Asserting that there is an unrealize would catch at least > some cases of "forgot to handle hotplug", though, right?
Yes. I figure it's worth doing. Another idea: track down all the instances of "parent class implements ::realize(), ::unrealize(), which in turn call methods its children may implement", and rename the children's methods to be called realize(), unrealize(), so that we can find offenders with a git-grep -E '->(un)?realize *='. >> > More generally, what is causing dc390 to be hotpluggable? >> > I can't see anything obvious in the class init. >> >> It's a PCI device. These are all hot-pluggable by default. > > Maybe we should change that? Most people writing a > PCI device are probably not thinking about hotplug. More on that below. > Though in the dc390 case it would probably not help to have > to set a dc->hotpluggable flag, because it would inherit that > from am53c974 too. > > I wonder if the dc390 could be structured in a way that > it doesn't subclass a complete non-abstract pci device like > that... Yes, a common abstract base class would be cleaner. >> > If we >> > have APIs where the default is "you get this weird thing >> > you weren't thinking about but it's broken because >> > you weren't thinking about it" then we will have a whole >> > class of bugs that we then need to squash device by device[*]. >> > I think it would be better for devices to have to explicitly >> > opt in to implementing things like hotplug -- that way the >> > failure mode is just "this isn't hotpluggable", we can >> > report that helpfully to the user, and if anybody cares >> > they can add the support. >> >> I tend to agree, although for PCI devices, not being hot-pluggable is >> kind of weird, except for the ones that only occur soldered onto >> mainboards. > > Real hardware PCI devices are not hot-pluggable by default > as far as I'm aware; PCIe may be hot-pluggable but I think > need to be designed to support it. > This is a random non-hotplug-safe PCIe card: > https://i.stack.imgur.com/tXug5.jpg > PRSNT2# (pin 17 side B) is connected to Ground on pin 18 > (these are the rightmost two connections visible); for > hotplug it must be connected to PRSNT1# (which is pin 1 side A), > AIUI. > > But the hardware situation is kind of a tangent from how > we try to design our APIs. It's best not to deviate from real hardware without a good reason. Making it harder to create the same stupid bugs over and over again is a good reason.
