On 3/17/2025 2:18 PM, Tony Lindgren wrote:
> Hi,
>
> On Mon, Mar 10, 2025 at 04:18:34PM +0800, Chenyi Qiang wrote:
>> --- a/system/physmem.c
>> +++ b/system/physmem.c
>> @@ -1885,6 +1886,16 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
>> qemu_mutex_unlock_ramlist();
>> goto out_free;
>> }
>> +
>> + new_block->memory_attribute_manager = MEMORY_ATTRIBUTE_MANAGER(object_new(TYPE_MEMORY_ATTRIBUTE_MANAGER));
>> + if (memory_attribute_manager_realize(new_block->memory_attribute_manager, new_block->mr)) {
>> + error_setg(errp, "Failed to realize memory attribute manager");
>> + object_unref(OBJECT(new_block->memory_attribute_manager));
>> + close(new_block->guest_memfd);
>> + ram_block_discard_require(false);
>> + qemu_mutex_unlock_ramlist();
>> + goto out_free;
>> + }
>> }
>>
>> ram_size = (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS;
>
> Might as well put the above into a separate memory manager init function
> to start with. It keeps the goto out_free error path unified, and makes
> things more future proof if the rest of ram_block_add() ever develops a
> need to check for errors.
Which part to be defined in a separate function? The init function of
object_new() + realize(), or the error handling operation
(object_unref() + close() + ram_block_discard_require(false))?
If need to check for errors in the rest of ram_block_add() in future,
how about adding a new label before out_free and move the error handling
there?
>
> Regards,
>
> Tony
