Re: [PATCH v6 09/12] softmmu/physmem: Don't use atomic operations in ram_block_discard_(disable|require)

[David Hildenbrand <david@redhat.com>]

On 22.02.21 15:02, Paolo Bonzini wrote:
> On 22/02/21 14:33, David Hildenbrand wrote:
>>> Also, uncoordinated require is unused, and therefore uncoordinated
>>> disable is also never going to block anything.  Does it make sense to
>>> keep it in the API?
>>
>> Right, "ram_block_discard_require()" is not used yet. I am planning on
>> using it in virtio-balloon context at some point, but can remove it for
>> now to simplify.
>>
>> ram_block_uncoordinated_discard_disable(), however, will block
>> virtio-balloon already via ram_block_discard_is_disabled(). (yes,
>> virtio-balloon is ugly)
> 
> Oops, I missed that API.
> 
> Does it make sense to turn the API inside out, with the
> coordinated/uncoordinated choice as an argument and the start/finish
> choice in the name?
> 
> enum {
>       RAM_DISCARD_ALLOW_COORDINATED = 1,
> };
> 

Any reason to go with an enum/flags for this case and not "bool 
allow_coordinated" ?

> bool ram_discard_disable(int flags, Error **errp);
> void ram_discard_enable(int flags);
> int ram_discard_start(bool coordinated, Error **errp);
> void ram_discard_finish(bool coordinated);

Yeah, I tried to avoid boolean flags ;) Don't have a strong opinion. At 
least we get shorter names.


-- 
Thanks,

David / dhildenb