memory: Propagate Error* when possible

[PATCH-for-9.0 01/25] memory: Have memory_region_init_ram_flags_nomigrate() return a boolean

Posted by Philippe Mathieu-Daudé 1 year ago

Following the example documented since commit e3fe3988d7 ("error:
Document Error API usage rules"), have cpu_exec_realizefn()
return a boolean indicating whether an error is set or not.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/exec/memory.h | 4 +++-
 system/memory.c       | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 831f7c996d..f038a7e5cf 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1310,8 +1310,10 @@ void memory_region_init_ram_nomigrate(MemoryRegion *mr,
  *
  * Note that this function does not do anything to cause the data in the
  * RAM memory region to be migrated; that is the responsibility of the caller.
+ *
+ * Return: true on success, else false setting @errp with error.
  */
-void memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
+bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
                                             Object *owner,
                                             const char *name,
                                             uint64_t size,
diff --git a/system/memory.c b/system/memory.c
index 4d9cb0a7ff..313dbb2544 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1583,7 +1583,7 @@ void memory_region_init_ram_nomigrate(MemoryRegion *mr,
     memory_region_init_ram_flags_nomigrate(mr, owner, name, size, 0, errp);
 }
 
-void memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
+bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
                                             Object *owner,
                                             const char *name,
                                             uint64_t size,
@@ -1600,7 +1600,9 @@ void memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
         mr->size = int128_zero();
         object_unparent(OBJECT(mr));
         error_propagate(errp, err);
+        return false;
     }
+    return true;
 }
 
 void memory_region_init_resizeable_ram(MemoryRegion *mr,
-- 
2.41.0

Re: [PATCH-for-9.0 01/25] memory: Have memory_region_init_ram_flags_nomigrate() return a boolean

Posted by Gavin Shan 11 months, 4 weeks ago

On 11/21/23 07:32, Philippe Mathieu-Daudé wrote:
> Following the example documented since commit e3fe3988d7 ("error:
> Document Error API usage rules"), have cpu_exec_realizefn()
> return a boolean indicating whether an error is set or not.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/exec/memory.h | 4 +++-
>   system/memory.c       | 4 +++-
>   2 files changed, 6 insertions(+), 2 deletions(-)
> 

With Peter Xu's comments addressed, to replace cpu_exec_realizefn()
with memory_region_init_ram_flags_nomigrate() in the commit log.

Reviewed-by: Gavin Shan <gshan@redhat.com>

Re: [PATCH-for-9.0 01/25] memory: Have memory_region_init_ram_flags_nomigrate() return a boolean

Posted by Peter Xu 1 year ago

On Mon, Nov 20, 2023 at 10:32:35PM +0100, Philippe Mathieu-Daudé wrote:
> Following the example documented since commit e3fe3988d7 ("error:
> Document Error API usage rules"), have cpu_exec_realizefn()

s/cpu_exec_realizefn/memory_region_init_ram_nomigrate/

Similar issue for initial 3 patches (or just not call out the function
name).

> return a boolean indicating whether an error is set or not.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Other than that:

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH-for-9.0 01/25] memory: Have memory_region_init_ram_flags_nomigrate() return a boolean

Posted by Manos Pitsidianakis 1 year ago

On Mon, 20 Nov 2023 23:32, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>Following the example documented since commit e3fe3988d7 ("error:
>Document Error API usage rules"), have cpu_exec_realizefn()
>return a boolean indicating whether an error is set or not.
>
>Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>---
> include/exec/memory.h | 4 +++-
> system/memory.c       | 4 +++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
>diff --git a/include/exec/memory.h b/include/exec/memory.h
>index 831f7c996d..f038a7e5cf 100644
>--- a/include/exec/memory.h
>+++ b/include/exec/memory.h
>@@ -1310,8 +1310,10 @@ void memory_region_init_ram_nomigrate(MemoryRegion *mr,
>  *
>  * Note that this function does not do anything to cause the data in the
>  * RAM memory region to be migrated; that is the responsibility of the caller.
>+ *
>+ * Return: true on success, else false setting @errp with error.
>  */
>-void memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
>+bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
>                                             Object *owner,
>                                             const char *name,
>                                             uint64_t size,
>diff --git a/system/memory.c b/system/memory.c
>index 4d9cb0a7ff..313dbb2544 100644
>--- a/system/memory.c
>+++ b/system/memory.c
>@@ -1583,7 +1583,7 @@ void memory_region_init_ram_nomigrate(MemoryRegion *mr,
>     memory_region_init_ram_flags_nomigrate(mr, owner, name, size, 0, errp);
> }
> 
>-void memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
>+bool memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
>                                             Object *owner,
>                                             const char *name,
>                                             uint64_t size,
>@@ -1600,7 +1600,9 @@ void memory_region_init_ram_flags_nomigrate(MemoryRegion *mr,
>         mr->size = int128_zero();
>         object_unparent(OBJECT(mr));
>         error_propagate(errp, err);
>+        return false;
>     }
>+    return true;
> }
> 
> void memory_region_init_resizeable_ram(MemoryRegion *mr,
>-- 
>2.41.0
>

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>