[PATCH-for-9.0 16/25] backends: Have HostMemoryBackendClass::alloc() handler return a boolean

Philippe Mathieu-Daudé posted 25 patches 1 year ago
Maintainers: David Hildenbrand <david@redhat.com>, Igor Mammedov <imammedo@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>, Jean-Christophe Dubois <jcd@tribudubois.net>, "Hervé Poussineau" <hpoussin@reactos.org>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Stefan Weil <sw@weilnetz.de>
[PATCH-for-9.0 16/25] backends: Have HostMemoryBackendClass::alloc() handler 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/sysemu/hostmem.h | 10 +++++++++-
 backends/hostmem-epc.c   | 11 +++++------
 backends/hostmem-file.c  | 19 ++++++++++---------
 backends/hostmem-memfd.c | 10 +++++-----
 backends/hostmem-ram.c   |  9 +++++----
 backends/hostmem.c       |  5 ++---
 6 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index 39326f1d4f..0e411aaa29 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -47,7 +47,15 @@ OBJECT_DECLARE_TYPE(HostMemoryBackend, HostMemoryBackendClass,
 struct HostMemoryBackendClass {
     ObjectClass parent_class;
 
-    void (*alloc)(HostMemoryBackend *backend, Error **errp);
+    /**
+     * alloc: Allocate memory from backend.
+     *
+     * @backend: the #HostMemoryBackend.
+     * @errp: pointer to Error*, to store an error if it happens.
+     *
+     * Return: true on success, else false setting @errp with error.
+     */
+    bool (*alloc)(HostMemoryBackend *backend, Error **errp);
 };
 
 /**
diff --git a/backends/hostmem-epc.c b/backends/hostmem-epc.c
index 3ceb079f9e..735e2e1cf8 100644
--- a/backends/hostmem-epc.c
+++ b/backends/hostmem-epc.c
@@ -17,7 +17,7 @@
 #include "sysemu/hostmem.h"
 #include "hw/i386/hostmem-epc.h"
 
-static void
+static bool
 sgx_epc_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 {
     g_autofree char *name = NULL;
@@ -26,21 +26,20 @@ sgx_epc_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 
     if (!backend->size) {
         error_setg(errp, "can't create backend with size 0");
-        return;
+        return false;
     }
 
     fd = qemu_open_old("/dev/sgx_vepc", O_RDWR);
     if (fd < 0) {
         error_setg_errno(errp, errno,
                          "failed to open /dev/sgx_vepc to alloc SGX EPC");
-        return;
+        return false;
     }
 
     name = object_get_canonical_path(OBJECT(backend));
     ram_flags = (backend->share ? RAM_SHARED : 0) | RAM_PROTECTED;
-    memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend),
-                                   name, backend->size, ram_flags,
-                                   fd, 0, errp);
+    return memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
+                                          backend->size, ram_flags, fd, 0, errp);
 }
 
 static void sgx_epc_backend_instance_init(Object *obj)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index fe8c481f8f..ac3e433cbd 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -36,12 +36,13 @@ struct HostMemoryBackendFile {
     OnOffAuto rom;
 };
 
-static void
+static bool
 file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 {
 #ifndef CONFIG_POSIX
     error_setg(errp, "backend '%s' not supported on this host",
                object_get_typename(OBJECT(backend)));
+    return false;
 #else
     HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
     g_autofree gchar *name = NULL;
@@ -49,11 +50,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 
     if (!backend->size) {
         error_setg(errp, "can't create backend with size 0");
-        return;
+        return false;
     }
     if (!fb->mem_path) {
         error_setg(errp, "mem-path property not set");
-        return;
+        return false;
     }
 
     switch (fb->rom) {
@@ -65,18 +66,18 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
         if (!fb->readonly) {
             error_setg(errp, "property 'rom' = 'on' is not supported with"
                        " 'readonly' = 'off'");
-            return;
+            return false;
         }
         break;
     case ON_OFF_AUTO_OFF:
         if (fb->readonly && backend->share) {
             error_setg(errp, "property 'rom' = 'off' is incompatible with"
                        " 'readonly' = 'on' and 'share' = 'on'");
-            return;
+            return false;
         }
         break;
     default:
-        assert(false);
+        g_assert_not_reached();
     }
 
     name = host_memory_backend_get_name(backend);
@@ -86,9 +87,9 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
     ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
     ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
     ram_flags |= RAM_NAMED_FILE;
-    memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
-                                     backend->size, fb->align, ram_flags,
-                                     fb->mem_path, fb->offset, errp);
+    return memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
+                                            backend->size, fb->align, ram_flags,
+                                            fb->mem_path, fb->offset, errp);
 #endif
 }
 
diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index db28ab5a56..3923ea9364 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -31,7 +31,7 @@ struct HostMemoryBackendMemfd {
     bool seal;
 };
 
-static void
+static bool
 memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 {
     HostMemoryBackendMemfd *m = MEMORY_BACKEND_MEMFD(backend);
@@ -41,7 +41,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 
     if (!backend->size) {
         error_setg(errp, "can't create backend with size 0");
-        return;
+        return false;
     }
 
     fd = qemu_memfd_create(TYPE_MEMORY_BACKEND_MEMFD, backend->size,
@@ -49,14 +49,14 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
                            F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL : 0,
                            errp);
     if (fd == -1) {
-        return;
+        return false;
     }
 
     name = host_memory_backend_get_name(backend);
     ram_flags = backend->share ? RAM_SHARED : 0;
     ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
-    memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
-                                   backend->size, ram_flags, fd, 0, errp);
+    return memory_region_init_ram_from_fd(&backend->mr, OBJECT(backend), name,
+                                          backend->size, ram_flags, fd, 0, errp);
 }
 
 static bool
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index 0a670fc22a..d121249f0f 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -16,7 +16,7 @@
 #include "qemu/module.h"
 #include "qom/object_interfaces.h"
 
-static void
+static bool
 ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 {
     g_autofree char *name = NULL;
@@ -24,14 +24,15 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
 
     if (!backend->size) {
         error_setg(errp, "can't create backend with size 0");
-        return;
+        return false;
     }
 
     name = host_memory_backend_get_name(backend);
     ram_flags = backend->share ? RAM_SHARED : 0;
     ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
-    memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend), name,
-                                           backend->size, ram_flags, errp);
+    return memory_region_init_ram_flags_nomigrate(&backend->mr, OBJECT(backend),
+                                                  name, backend->size,
+                                                  ram_flags, errp);
 }
 
 static void
diff --git a/backends/hostmem.c b/backends/hostmem.c
index 1723c19165..3f8eb936d7 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -331,9 +331,8 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
     if (!bc->alloc) {
         return;
     }
-    bc->alloc(backend, &local_err);
-    if (local_err) {
-        goto out;
+    if (!bc->alloc(backend, errp)) {
+        return;
     }
 
     ptr = memory_region_get_ram_ptr(&backend->mr);
-- 
2.41.0


Re: [PATCH-for-9.0 16/25] backends: Have HostMemoryBackendClass::alloc() handler 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/sysemu/hostmem.h | 10 +++++++++-
>   backends/hostmem-epc.c   | 11 +++++------
>   backends/hostmem-file.c  | 19 ++++++++++---------
>   backends/hostmem-memfd.c | 10 +++++-----
>   backends/hostmem-ram.c   |  9 +++++----
>   backends/hostmem.c       |  5 ++---
>   6 files changed, 36 insertions(+), 28 deletions(-)
> 

s/cpu_exec_realizefn()/HostMemoryBackendClass::alloc()

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


Re: [PATCH-for-9.0 16/25] backends: Have HostMemoryBackendClass::alloc() handler 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>
>---

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