On 23/05/2017 17:28, Greg Kurz wrote:
> On Tue, 23 May 2017 13:18:09 +0200
> Laurent Vivier <lviv...@redhat.com> wrote:
>
>> This allows to manage errors before the memory
>> has started to be hotplugged. We already have
>> the function for the CPU cores.
>>
>> Signed-off-by: Laurent Vivier <lviv...@redhat.com>
>> ---
>> hw/ppc/spapr.c | 45 ++++++++++++++++++++++++++++++---------------
>> 1 file changed, 30 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 0980d73..0e8d8d1 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -2569,20 +2569,6 @@ static void spapr_memory_plug(HotplugHandler
>> *hotplug_dev, DeviceState *dev,
>> uint64_t align = memory_region_get_alignment(mr);
>> uint64_t size = memory_region_size(mr);
>> uint64_t addr;
>> - char *mem_dev;
>> -
>> - if (size % SPAPR_MEMORY_BLOCK_SIZE) {
>> - error_setg(&local_err, "Hotplugged memory size must be a multiple
>> of "
>> - "%lld MB", SPAPR_MEMORY_BLOCK_SIZE/M_BYTE);
>> - goto out;
>> - }
>> -
>> - mem_dev = object_property_get_str(OBJECT(dimm), PC_DIMM_MEMDEV_PROP,
>> NULL);
>> - if (mem_dev && !kvmppc_is_mem_backend_page_size_ok(mem_dev)) {
>> - error_setg(&local_err, "Memory backend has bad page size. "
>> - "Use 'memory-backend-file' with correct mem-path.");
>> - goto out;
>> - }
>>
>> pc_dimm_memory_plug(dev, &ms->hotplug_memory, mr, align, &local_err);
>> if (local_err) {
>> @@ -2603,6 +2589,33 @@ out:
>> error_propagate(errp, local_err);
>> }
>>
>> +static void spapr_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState
>> *dev,
>> + Error **errp)
>
> Indentation nit
ok
>
>> +{
>> + PCDIMMDevice *dimm = PC_DIMM(dev);
>> + PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(dimm);
>> + MemoryRegion *mr = ddc->get_memory_region(dimm);
>> + uint64_t size = memory_region_size(mr);
>> + Error *local_err = NULL;
>> + char *mem_dev;
>> +
>> + if (size % SPAPR_MEMORY_BLOCK_SIZE) {
>> + error_setg(&local_err, "Hotplugged memory size must be a multiple
>> of "
>> + "%lld MB", SPAPR_MEMORY_BLOCK_SIZE / M_BYTE);
>> + goto out;
>> + }
>> +
>> + mem_dev = object_property_get_str(OBJECT(dimm), PC_DIMM_MEMDEV_PROP,
>> NULL);
>> + if (mem_dev && !kvmppc_is_mem_backend_page_size_ok(mem_dev)) {
>> + error_setg(&local_err, "Memory backend has bad page size. "
>> + "Use 'memory-backend-file' with correct mem-path.");
>> + goto out;
>> + }
>> +
>> +out:
>> + error_propagate(errp, local_err);
>
> As recently discussed with Markus Armbruster, it isn't necessary to have a
> local Error * if you don't do anything else with it but propagate it.
Yes, you are right, it's a stupid cut'n'paste.
Thanks,
Laurent
