Re: [PATCH v2 fixed 01/16] util: vfio-helpers: Factor out and fix processing of existing ram blocks

[David Hildenbrand]

On 19.02.20 09:43, David Hildenbrand wrote:
> On 18.02.20 23:00, Peter Xu wrote:
>> On Wed, Feb 12, 2020 at 02:42:39PM +0100, David Hildenbrand wrote:
>>> Factor it out into common code when a new notifier is registered, just
>>> as done with the memory region notifier. This allows us to have the
>>> logic about how to process existing ram blocks at a central place (which
>>> will be extended soon).
>>>
>>> Just like when adding a new ram block, we have to register the max_length
>>> for now. We don't have a way to get notified about resizes yet, and some
>>> memory would not be mapped when growing the ram block.
>>>
>>> Note: Currently, ram blocks are only "fake resized". All memory
>>> (max_length) is accessible.
>>>
>>> We can get rid of a bunch of functions in stubs/ram-block.c . Print the
>>> warning from inside qemu_vfio_ram_block_added().
> 
> [...]
> 
>>>  #include "exec/ramlist.h"
>>>  #include "exec/cpu-common.h"
>>>  
>>> -void *qemu_ram_get_host_addr(RAMBlock *rb)
>>> -{
>>> -    return 0;
>>> -}
>>> -
>>> -ram_addr_t qemu_ram_get_offset(RAMBlock *rb)
>>> -{
>>> -    return 0;
>>> -}
>>> -
>>> -ram_addr_t qemu_ram_get_used_length(RAMBlock *rb)
>>> -{
>>> -    return 0;
>>> -}
>>
>> Maybe put into another patch?
>>
>> Actually I'm thinking whether it would worth to do...  They're still
>> declared in include/exec/cpu-common.h, so logically who includes the
>> header but linked against stubs can still call this function.  So
>> keeping them there still make sense to me.
> 
> Why keep dead code around? If you look closely, the stubs really only
> contain what's strictly necessary to make current code compile, not any
> available ramblock related function.
> 
> I don't see a good reason for a separate patch either (after all, we're
> removing the last users in this patch), but if more people agree, I can
> move it to a separate patch.

FWIW, moved it to a separate patch :)


-- 
Thanks,

David / dhildenb