Re: [PATCH v5 08/10] memory: Change NotifyRamDiscard() definition to return the result

[Chenyi Qiang <chenyi.qiang@intel.com>]

On 5/26/2025 8:44 PM, Cédric Le Goater wrote:
> On 5/26/25 12:36, Cédric Le Goater wrote:
>> On 5/20/25 12:28, Chenyi Qiang wrote:
>>> So that the caller can check the result of NotifyRamDiscard() handler if
>>> the operation fails.
>>>
>>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com>
>>> ---
>>> Changes in v5:
>>>      - Revert to use of NotifyRamDiscard()
>>>
>>> Changes in v4:
>>>      - Newly added.
>>> ---
>>>   hw/vfio/listener.c           | 6 ++++--
>>>   include/system/memory.h      | 4 ++--
>>>   system/ram-block-attribute.c | 3 +--
>>>   3 files changed, 7 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/hw/vfio/listener.c b/hw/vfio/listener.c
>>> index bfacb3d8d9..06454e0584 100644
>>> --- a/hw/vfio/listener.c
>>> +++ b/hw/vfio/listener.c
>>> @@ -190,8 +190,8 @@ out:
>>>       rcu_read_unlock();
>>>   }
>>> -static void vfio_ram_discard_notify_discard(RamDiscardListener *rdl,
>>> -                                            MemoryRegionSection
>>> *section)
>>> +static int vfio_ram_discard_notify_discard(RamDiscardListener *rdl,
>>> +                                           MemoryRegionSection
>>> *section)
>>>   {
>>>       VFIORamDiscardListener *vrdl = container_of(rdl,
>>> VFIORamDiscardListener,
>>>                                                   listener);
>>> @@ -206,6 +206,8 @@ static void
>>> vfio_ram_discard_notify_discard(RamDiscardListener *rdl,
>>>           error_report("%s: vfio_container_dma_unmap() failed: %s",
>>> __func__,
>>>                        strerror(-ret));
>>>       }
>>> +
>>> +    return ret;
>>>   }
>>
>> vfio_ram_discard_notify_populate() should also be modified
>> to return this value.
> 
> Nope. It should not. This is a rollback path in case of error. All good.

Thanks for your review! Anyway, according to the discussion in patch
#10, I'll revert this patch in next version, since it is a future work
to consider the failure case of notifying discard.

> 
> Thanks,
> 
> C.
>