On Sun, 25 Oct 2020 11:13:40 +0100 Greg Kurz <gr...@kaod.org> wrote: > On Thu, 22 Oct 2020 15:11:42 +1100 > David Gibson <da...@gibson.dropbear.id.au> wrote: > > > On Mon, Oct 19, 2020 at 10:47:52AM +0200, Greg Kurz wrote: > > > Hi, > > > > > > This is a followup to a previous cleanup for the sPAPR code: > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2020-09/msg04860.html > > > > > > The last two patches had to be dropped because they were wrongly assuming > > > that object_property_get_uint() returning zero meant failure. This led to > > > a discussion in which arose a consensus that most of the time (not to say > > > always) object property getters should never fail actually, ie. failure > > > is very likely the result of a programming error and QEMU should abort. > > > > > > This series aims at demonstrating a revelant case I've found while > > > auditing > > > object property getters (this is patch 4 that I've isolated from a huge > > > 50-patch series I haven't dared to post yet). The sPAPR memory hotplug > > > code > > > is tailored to support either regular PC DIMMs or NVDIMMs, which inherit > > > from PC DIMMs. They expect to get some properties from the DIMM object, > > > which happens to be set by default at the PC DIMM class level. It thus > > > doesn't make sense to pass an error object and propagate it when getting > > > them since this would lure the user into thinking they did something > > > wrong. > > > > > > Some preliminary cleanup is done on the way, especially dropping an unused > > > @errp argument of pc_dimm_plug(). This affects several platforms other > > > than > > > sPAPR but I guess the patch is trivial enough to go through David's tree > > > if it gets acks from the relevant maintainers. > > > > Since this series mostly affects ppc, I've applied it to ppc-for-5.2. > > > > It would be nice to have an acked-by from Igor or Michael for the > > first patch, though. > > > > David, > > Igor sent a R-b for patches 1 and 4. He also suggested to call > spapr_drc_attach() at pre-plug time. I'll look into this, so maybe > you can drop patch 5 from ppc-for-5.2 (or the entire series at > your convenience). >
It seems that spapr_drc_attach() cannot be called at pre-plug time actually because there is no way to call spapr_drc_detach() if the device fails to realize. I think you there's nothing else to do for this series than adding Igor's r-b to patches 1 and 4. > Cheers, > > -- > Greg > > > > > > > --- > > > > > > Greg Kurz (5): > > > pc-dimm: Drop @errp argument of pc_dimm_plug() > > > spapr: Use appropriate getter for PC_DIMM_ADDR_PROP > > > spapr: Use appropriate getter for PC_DIMM_SLOT_PROP > > > spapr: Pass &error_abort when getting some PC DIMM properties > > > spapr: Simplify error handling in spapr_memory_plug() > > > > > > > > > hw/arm/virt.c | 9 +------- > > > hw/i386/pc.c | 8 +------ > > > hw/mem/pc-dimm.c | 2 +- > > > hw/ppc/spapr.c | 48 > > > +++++++++++++++-------------------------- > > > hw/ppc/spapr_nvdimm.c | 5 +++- > > > include/hw/mem/pc-dimm.h | 2 +- > > > include/hw/ppc/spapr_nvdimm.h | 2 +- > > > 7 files changed, 25 insertions(+), 51 deletions(-) > > > > > >
pgpTsTnTg4w48.pgp
Description: OpenPGP digital signature