Re: [Qemu-devel] [PATCH v2] hw/ppc/spapr: Fix segfault when instantiating a 'pc-dimm' without 'memdev'

[Pankaj Gupta <pagupta@redhat.com>]

> > 
> > Hello Thomas,
> >>
> >> QEMU currently crashes when trying to use a 'pc-dimm' on the pseries
> >> machine without specifying its 'memdev' property. This happens because
> >> pc_dimm_get_memory_region() does not check whether the 'memdev' property
> >> has properly been set by the user. Looking closer at this function, it's
> >> also obvious that it is using &error_abort to call another function - and
> >> this is bad in a function that is used in the hot-plugging calling chain
> >> since this can also cause QEMU to exit unexpectedly.
> > 
> > In place of checking if 'memdev' is already allocated/present while
> > fetching
> > 'get_functions' every place. If 'memdev' is required for memory hotplug
> > operation
> > can we just put a check at some common place and don't start or instantiate
> > if
> > 'memdev' is not present.
> > 
> > I can see:
> > 
> > 'pc_dimm_realize' function checks for : in my case prints warning
> > and avoid Qemu to start.
> > ...
> > ...
> > if (!dimm->hostmem) {
> >         error_setg(errp, "'" PC_DIMM_MEMDEV_PROP "' property is not set");
> >         return;
> >     }
> > ...
> 
> That check unfortunately does not apply here: The crash happens in the
> pre_plug() handler of the spapr machine, and this is called *before* the
> realize function of the pc-dimm is called!

o.k I guessed so.
> 
> But even if we could find another common place where we could check
> dimm->hostmem first, you still have the problem that
> host_memory_backend_get_memory() could return an error and thus
> pc_dimm_get_memory_region() still needs a way to fail gracefully during
> the hotplug (or also hot-unplug) operation. So I think my patch is the
> right way to go here.

Right. Thanks for the explanation.

Pankaj
> 
>  Thomas
>