[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to acc
From: |
David Hildenbrand |
Subject: |
Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized |
Date: |
Fri, 15 Jun 2018 11:25:59 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0 |
On 15.06.2018 11:16, Igor Mammedov wrote:
> On Thu, 14 Jun 2018 16:10:14 +0200
> David Hildenbrand <address@hidden> wrote:
>
>> On 14.06.2018 15:24, Igor Mammedov wrote:
>>> On Wed, 13 Jun 2018 16:03:35 +0200
>>> David Hildenbrand <address@hidden> wrote:
>>>
>>>> On 13.06.2018 14:56, Igor Mammedov wrote:
>>>>> On Mon, 11 Jun 2018 14:16:50 +0200
>>>>> David Hildenbrand <address@hidden> wrote:
>>>>>
>>>>>> "size" should not be queried before the device was realized. Let' make
>>>>>> that explicit.
>>>>>>
>>>>>> Signed-off-by: David Hildenbrand <address@hidden>
>>>>> It's generic property getter, it should work even before realize is
>>>>> called.
>>>>
>>>> That's the point, as we can see in NVDIMM code , it is *not* a generic
>>>> property getter.
>>> it is added by
>>> object_property_add(obj, PC_DIMM_SIZE_PROP, "uint64", pc_dimm_get_size,
>>>
>>> NULL, NULL, NULL, &error_abort);
>>> hence it's generic QOM property as opposed to memory device class specific
>>> callbacks.
>>>
>>>>
>>>>>
>>>>> if issues described in 5/11 are properly fixed, this patch won't be
>>>>> needed.
>>>>>
>>>>> So drop this patch
>>>>>
>>>>>
>>>>>> ---
>>>>>> hw/mem/pc-dimm.c | 6 ++++++
>>>>>> 1 file changed, 6 insertions(+)
>>>>>>
>>>>>> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
>>>>>> index 86fbcf2d0c..5294734529 100644
>>>>>> --- a/hw/mem/pc-dimm.c
>>>>>> +++ b/hw/mem/pc-dimm.c
>>>>>> @@ -166,6 +166,12 @@ static void pc_dimm_get_size(Object *obj, Visitor
>>>>>> *v, const char *name,
>>>>>> PCDIMMDevice *dimm = PC_DIMM(obj);
>>>>>> PCDIMMDeviceClass *ddc = PC_DIMM_GET_CLASS(obj);
>>>>>>
>>>>>> + if (!DEVICE(obj)->realized) {
>>>>>> + error_setg(errp, "Property \"%s\" not accessible before
>>>>>> realized",
>>>>>> + name);
>>>>>> + return;
>>>>>> + }
>>>>>> +
>>>>>> mr = ddc->get_memory_region(dimm, errp);
>>> just make ddc->get_memory_region return error if 'memdev' isn't set yet
>>> and add local_error handling here as proper property assessor should do.
>>>
>>
>> Does not help for NVDIMM.
> NVDIMM should be fixed as it's broken now and works just by accident.
> I don't like tailoring infrastructure changes to a particular broken
> device especially when the later could be made to work within current
> framework.
>
Yes, but I will go the path of static properties for NVDIMM instead.
Trying to initialize the memory region in a property setter
("label-size") and then bailing out because another property (memdev) is
not set looks strange. I will do checks+initialization in realize() and
in get_memory_region(). This looks better in my point of view and looks
like a nice NVDIMM refactoring (at least judging from the patches I have
on my list now :) ).
--
Thanks,
David / dhildenb
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 03/11] pc: factor out pc-dimm checks into pc_dimm_pre_plug(), (continued)
- [Qemu-ppc] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized, David Hildenbrand, 2018/06/11
- Re: [Qemu-ppc] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized, David Gibson, 2018/06/11
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized, Igor Mammedov, 2018/06/13
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized, Igor Mammedov, 2018/06/14
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized, David Hildenbrand, 2018/06/14
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized, Igor Mammedov, 2018/06/15
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized,
David Hildenbrand <=
- Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 06/11] pc-dimm: don't allow to access "size" before the device was realized, Igor Mammedov, 2018/06/15
[Qemu-ppc] [PATCH v1 05/11] spapr: move memory hotplug size check into plug code, David Hildenbrand, 2018/06/11
[Qemu-ppc] [PATCH v1 04/11] hostmem: drop error variable from host_memory_backend_get_memory(), David Hildenbrand, 2018/06/11