Re: [Qemu-ppc] [Qemu-devel] [PATCH v1 07/11] pc-dimm: get_memory_region() can never fail

[David Hildenbrand]

On 13.06.2018 15:03, Igor Mammedov wrote:
> On Mon, 11 Jun 2018 14:16:51 +0200
> David Hildenbrand <address@hidden> wrote:
> 
>> We already verify when realizing that the memdev property has been
>> set. We have no more accesses to get_memory_region() before the device
>> is realized.
> this stems from assumption that get_memory_region shouldn't be called
> before devices realize is executed, which I don't agree to begin with.
> 
> However wrt error handling, we should probably leave device internal error
> up to devices and make check for error only in pre_plug handler
> (since pre_plug was successful, the rest machine helpers would have
> access to the same region until device is removed.
> 

Something like a generic Device "validate()"/"pre_realize()" function,
that can be called before realize() and validates all properties
(+initializes derived properties) would be something I could agree to.

Then we could drop all error handling from access functions (as they
have been validated early during pre_plug())

Moving all checks out of realize into pre_plug() looks ugly, because we
have implementation details split across c-files.

> 
>> So this function will never fail. Remove the stale check and the
>> error variable. Add a comment to the functions stating that they should
>> never be called on uninitialized devices.
>>


-- 

Thanks,

David / dhildenb