Let's provide a way to control the use of RAM_NORESERVE via memory
backends using the "reserve" property which defaults to true (old
behavior).
Only Linux currently supports clearing the flag (and support is checked at
runtime, depending on the setting of "/proc/sys/vm/overcommit_memory").
Windows and other POSIX systems will bail out with "reserve=false".
The target use case is virtio-mem, which dynamically exposes memory
inside a large, sparse memory area to the VM. This essentially allows
avoiding to set "/proc/sys/vm/overcommit_memory == 0") when using
virtio-mem and also supporting hugetlbfs in the future.
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Acked-by: Eduardo Habkost <ehabkost@redhat.com> for memory backend and machine core
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Eric Blake <eblake@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
backends/hostmem-file.c | 11 ++++++-----
backends/hostmem-memfd.c | 1 +
backends/hostmem-ram.c | 1 +
backends/hostmem.c | 32 ++++++++++++++++++++++++++++++++
include/sysemu/hostmem.h | 2 +-
qapi/qom.json | 10 ++++++++++
6 files changed, 51 insertions(+), 6 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index b683da9daf..9d550e53d4 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -40,6 +40,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
object_get_typename(OBJECT(backend)));
#else
HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
+ uint32_t ram_flags;
gchar *name;
if (!backend->size) {
@@ -52,11 +53,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
}
name = host_memory_backend_get_name(backend);
- memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
- name,
- backend->size, fb->align,
- (backend->share ? RAM_SHARED : 0) |
- (fb->is_pmem ? RAM_PMEM : 0),
+ ram_flags = backend->share ? RAM_SHARED : 0;
+ ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
+ ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
+ memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
+ backend->size, fb->align, ram_flags,
fb->mem_path, fb->readonly, errp);
g_free(name);
#endif
diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
index 93b5d1a4cf..f3436b623d 100644
--- a/backends/hostmem-memfd.c
+++ b/backends/hostmem-memfd.c
@@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
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);
g_free(name);
diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
index 741e701062..b8e55cdbd0 100644
--- a/backends/hostmem-ram.c
+++ b/backends/hostmem-ram.c
@@ -29,6 +29,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
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);
g_free(name);
diff --git a/backends/hostmem.c b/backends/hostmem.c
index c6c1ff5b99..58fdc1b658 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -217,6 +217,11 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
Error *local_err = NULL;
HostMemoryBackend *backend = MEMORY_BACKEND(obj);
+ if (!backend->reserve && value) {
+ error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
+ return;
+ }
+
if (!host_memory_backend_mr_inited(backend)) {
backend->prealloc = value;
return;
@@ -268,6 +273,7 @@ static void host_memory_backend_init(Object *obj)
/* TODO: convert access to globals to compat properties */
backend->merge = machine_mem_merge(machine);
backend->dump = machine_dump_guest_core(machine);
+ backend->reserve = true;
backend->prealloc_threads = 1;
}
@@ -426,6 +432,28 @@ static void host_memory_backend_set_share(Object *o, bool value, Error **errp)
backend->share = value;
}
+static bool host_memory_backend_get_reserve(Object *o, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(o);
+
+ return backend->reserve;
+}
+
+static void host_memory_backend_set_reserve(Object *o, bool value, Error **errp)
+{
+ HostMemoryBackend *backend = MEMORY_BACKEND(o);
+
+ if (host_memory_backend_mr_inited(backend)) {
+ error_setg(errp, "cannot change property value");
+ return;
+ }
+ if (backend->prealloc && !value) {
+ error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
+ return;
+ }
+ backend->reserve = value;
+}
+
static bool
host_memory_backend_get_use_canonical_path(Object *obj, Error **errp)
{
@@ -494,6 +522,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
host_memory_backend_get_share, host_memory_backend_set_share);
object_class_property_set_description(oc, "share",
"Mark the memory as private to QEMU or shared");
+ object_class_property_add_bool(oc, "reserve",
+ host_memory_backend_get_reserve, host_memory_backend_set_reserve);
+ object_class_property_set_description(oc, "reserve",
+ "Reserve swap space (or huge pages) if applicable");
/*
* Do not delete/rename option. This option must be considered stable
* (as if it didn't have the 'x-' prefix including deprecation period) as
diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
index df5644723a..9ff5c16963 100644
--- a/include/sysemu/hostmem.h
+++ b/include/sysemu/hostmem.h
@@ -64,7 +64,7 @@ struct HostMemoryBackend {
/* protected */
uint64_t size;
bool merge, dump, use_canonical_path;
- bool prealloc, is_mapped, share;
+ bool prealloc, is_mapped, share, reserve;
uint32_t prealloc_threads;
DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
HostMemPolicy policy;
diff --git a/qapi/qom.json b/qapi/qom.json
index cd0e76d564..4fa3137aab 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -545,6 +545,9 @@
# @share: if false, the memory is private to QEMU; if true, it is shared
# (default: false)
#
+# @reserve: if true, reserve swap space (or huge pages) if applicable
+# default: true)
+#
# @size: size of the memory region in bytes
#
# @x-use-canonical-path-for-ramblock-id: if true, the canoncial path is used
@@ -556,6 +559,12 @@
# false generally, but true for machine
# types <= 4.0)
#
+# Note: prealloc=true and reserve=false cannot be set at the same time. With
+# reserve=true, the behavior depends on the operating system: for example,
+# Linux will not reserve swap space for shared file mappings --
+# "not applicable". In contrast, reserve=false will bail out if it cannot
+# be configured accordingly.
+#
# Since: 2.1
##
{ 'struct': 'MemoryBackendProperties',
@@ -566,6 +575,7 @@
'*prealloc': 'bool',
'*prealloc-threads': 'uint32',
'*share': 'bool',
+ '*reserve': 'bool',
'size': 'size',
'*x-use-canonical-path-for-ramblock-id': 'bool' } }
--
2.30.2
On 28/04/21 15:37, David Hildenbrand wrote: > @@ -545,6 +545,9 @@ > # @share: if false, the memory is private to QEMU; if true, it is shared > # (default: false) > # > +# @reserve: if true, reserve swap space (or huge pages) if applicable > +# default: true) Missing open parenthesis and "since 6.1", otherwise the whole series looks good, thanks! Paolo > +#
On 04.05.21 11:58, Paolo Bonzini wrote: > On 28/04/21 15:37, David Hildenbrand wrote: >> @@ -545,6 +545,9 @@ >> # @share: if false, the memory is private to QEMU; if true, it is shared >> # (default: false) >> # >> +# @reserve: if true, reserve swap space (or huge pages) if applicable >> +# default: true) > > Missing open parenthesis and "since 6.1", otherwise the whole series > looks good, thanks! I could have sworn I had the "since 6.1" in before :) Thanks! -- Thanks, David / dhildenb
On Wed, Apr 28, 2021 at 03:37:49PM +0200, David Hildenbrand wrote:
> Let's provide a way to control the use of RAM_NORESERVE via memory
> backends using the "reserve" property which defaults to true (old
> behavior).
>
> Only Linux currently supports clearing the flag (and support is checked at
> runtime, depending on the setting of "/proc/sys/vm/overcommit_memory").
> Windows and other POSIX systems will bail out with "reserve=false".
>
> The target use case is virtio-mem, which dynamically exposes memory
> inside a large, sparse memory area to the VM. This essentially allows
> avoiding to set "/proc/sys/vm/overcommit_memory == 0") when using
> virtio-mem and also supporting hugetlbfs in the future.
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Acked-by: Eduardo Habkost <ehabkost@redhat.com> for memory backend and machine core
> Cc: Markus Armbruster <armbru@redhat.com>
> Cc: Eric Blake <eblake@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> backends/hostmem-file.c | 11 ++++++-----
> backends/hostmem-memfd.c | 1 +
> backends/hostmem-ram.c | 1 +
> backends/hostmem.c | 32 ++++++++++++++++++++++++++++++++
> include/sysemu/hostmem.h | 2 +-
> qapi/qom.json | 10 ++++++++++
> 6 files changed, 51 insertions(+), 6 deletions(-)
>
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index b683da9daf..9d550e53d4 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -40,6 +40,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> object_get_typename(OBJECT(backend)));
> #else
> HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
> + uint32_t ram_flags;
> gchar *name;
>
> if (!backend->size) {
> @@ -52,11 +53,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> }
>
> name = host_memory_backend_get_name(backend);
> - memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> - name,
> - backend->size, fb->align,
> - (backend->share ? RAM_SHARED : 0) |
> - (fb->is_pmem ? RAM_PMEM : 0),
> + ram_flags = backend->share ? RAM_SHARED : 0;
> + ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
> + ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
> + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
> + backend->size, fb->align, ram_flags,
> fb->mem_path, fb->readonly, errp);
> g_free(name);
> #endif
> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> index 93b5d1a4cf..f3436b623d 100644
> --- a/backends/hostmem-memfd.c
> +++ b/backends/hostmem-memfd.c
> @@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>
> 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);
> g_free(name);
> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> index 741e701062..b8e55cdbd0 100644
> --- a/backends/hostmem-ram.c
> +++ b/backends/hostmem-ram.c
> @@ -29,6 +29,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>
> 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);
> g_free(name);
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index c6c1ff5b99..58fdc1b658 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -217,6 +217,11 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
> Error *local_err = NULL;
> HostMemoryBackend *backend = MEMORY_BACKEND(obj);
>
> + if (!backend->reserve && value) {
> + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
> + return;
> + }
> +
> if (!host_memory_backend_mr_inited(backend)) {
> backend->prealloc = value;
> return;
> @@ -268,6 +273,7 @@ static void host_memory_backend_init(Object *obj)
> /* TODO: convert access to globals to compat properties */
> backend->merge = machine_mem_merge(machine);
> backend->dump = machine_dump_guest_core(machine);
> + backend->reserve = true;
> backend->prealloc_threads = 1;
> }
>
> @@ -426,6 +432,28 @@ static void host_memory_backend_set_share(Object *o, bool value, Error **errp)
> backend->share = value;
> }
>
> +static bool host_memory_backend_get_reserve(Object *o, Error **errp)
> +{
> + HostMemoryBackend *backend = MEMORY_BACKEND(o);
> +
> + return backend->reserve;
> +}
> +
> +static void host_memory_backend_set_reserve(Object *o, bool value, Error **errp)
> +{
> + HostMemoryBackend *backend = MEMORY_BACKEND(o);
> +
> + if (host_memory_backend_mr_inited(backend)) {
> + error_setg(errp, "cannot change property value");
> + return;
> + }
> + if (backend->prealloc && !value) {
> + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
> + return;
> + }
> + backend->reserve = value;
> +}
> +
> static bool
> host_memory_backend_get_use_canonical_path(Object *obj, Error **errp)
> {
> @@ -494,6 +522,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
> host_memory_backend_get_share, host_memory_backend_set_share);
> object_class_property_set_description(oc, "share",
> "Mark the memory as private to QEMU or shared");
> + object_class_property_add_bool(oc, "reserve",
> + host_memory_backend_get_reserve, host_memory_backend_set_reserve);
> + object_class_property_set_description(oc, "reserve",
> + "Reserve swap space (or huge pages) if applicable");
> /*
> * Do not delete/rename option. This option must be considered stable
> * (as if it didn't have the 'x-' prefix including deprecation period) as
> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
> index df5644723a..9ff5c16963 100644
> --- a/include/sysemu/hostmem.h
> +++ b/include/sysemu/hostmem.h
> @@ -64,7 +64,7 @@ struct HostMemoryBackend {
> /* protected */
> uint64_t size;
> bool merge, dump, use_canonical_path;
> - bool prealloc, is_mapped, share;
> + bool prealloc, is_mapped, share, reserve;
> uint32_t prealloc_threads;
> DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
> HostMemPolicy policy;
> diff --git a/qapi/qom.json b/qapi/qom.json
> index cd0e76d564..4fa3137aab 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -545,6 +545,9 @@
> # @share: if false, the memory is private to QEMU; if true, it is shared
> # (default: false)
> #
> +# @reserve: if true, reserve swap space (or huge pages) if applicable
> +# default: true)
> +#
> # @size: size of the memory region in bytes
> #
> # @x-use-canonical-path-for-ramblock-id: if true, the canoncial path is used
> @@ -556,6 +559,12 @@
> # false generally, but true for machine
> # types <= 4.0)
> #
> +# Note: prealloc=true and reserve=false cannot be set at the same time. With
> +# reserve=true, the behavior depends on the operating system: for example,
> +# Linux will not reserve swap space for shared file mappings --
> +# "not applicable". In contrast, reserve=false will bail out if it cannot
> +# be configured accordingly.
> +#
> # Since: 2.1
> ##
> { 'struct': 'MemoryBackendProperties',
> @@ -566,6 +575,7 @@
> '*prealloc': 'bool',
> '*prealloc-threads': 'uint32',
> '*share': 'bool',
> + '*reserve': 'bool',
> 'size': 'size',
> '*x-use-canonical-path-for-ramblock-id': 'bool' } }
IIUC from the previous patch in the series, 'reserve' is only implemented
on Linux. If we make this QAPI prop dependant on CONFIG_LINUX, then mgmt
apps will do the right thing to detect what platform(s) it works on.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 04.05.21 12:12, Daniel P. Berrangé wrote:
> On Wed, Apr 28, 2021 at 03:37:49PM +0200, David Hildenbrand wrote:
>> Let's provide a way to control the use of RAM_NORESERVE via memory
>> backends using the "reserve" property which defaults to true (old
>> behavior).
>>
>> Only Linux currently supports clearing the flag (and support is checked at
>> runtime, depending on the setting of "/proc/sys/vm/overcommit_memory").
>> Windows and other POSIX systems will bail out with "reserve=false".
>>
>> The target use case is virtio-mem, which dynamically exposes memory
>> inside a large, sparse memory area to the VM. This essentially allows
>> avoiding to set "/proc/sys/vm/overcommit_memory == 0") when using
>> virtio-mem and also supporting hugetlbfs in the future.
>>
>> Reviewed-by: Peter Xu <peterx@redhat.com>
>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>> Acked-by: Eduardo Habkost <ehabkost@redhat.com> for memory backend and machine core
>> Cc: Markus Armbruster <armbru@redhat.com>
>> Cc: Eric Blake <eblake@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>> backends/hostmem-file.c | 11 ++++++-----
>> backends/hostmem-memfd.c | 1 +
>> backends/hostmem-ram.c | 1 +
>> backends/hostmem.c | 32 ++++++++++++++++++++++++++++++++
>> include/sysemu/hostmem.h | 2 +-
>> qapi/qom.json | 10 ++++++++++
>> 6 files changed, 51 insertions(+), 6 deletions(-)
>>
>> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
>> index b683da9daf..9d550e53d4 100644
>> --- a/backends/hostmem-file.c
>> +++ b/backends/hostmem-file.c
>> @@ -40,6 +40,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>> object_get_typename(OBJECT(backend)));
>> #else
>> HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
>> + uint32_t ram_flags;
>> gchar *name;
>>
>> if (!backend->size) {
>> @@ -52,11 +53,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>> }
>>
>> name = host_memory_backend_get_name(backend);
>> - memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>> - name,
>> - backend->size, fb->align,
>> - (backend->share ? RAM_SHARED : 0) |
>> - (fb->is_pmem ? RAM_PMEM : 0),
>> + ram_flags = backend->share ? RAM_SHARED : 0;
>> + ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
>> + ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
>> + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
>> + backend->size, fb->align, ram_flags,
>> fb->mem_path, fb->readonly, errp);
>> g_free(name);
>> #endif
>> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
>> index 93b5d1a4cf..f3436b623d 100644
>> --- a/backends/hostmem-memfd.c
>> +++ b/backends/hostmem-memfd.c
>> @@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>
>> 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);
>> g_free(name);
>> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
>> index 741e701062..b8e55cdbd0 100644
>> --- a/backends/hostmem-ram.c
>> +++ b/backends/hostmem-ram.c
>> @@ -29,6 +29,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>
>> 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);
>> g_free(name);
>> diff --git a/backends/hostmem.c b/backends/hostmem.c
>> index c6c1ff5b99..58fdc1b658 100644
>> --- a/backends/hostmem.c
>> +++ b/backends/hostmem.c
>> @@ -217,6 +217,11 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
>> Error *local_err = NULL;
>> HostMemoryBackend *backend = MEMORY_BACKEND(obj);
>>
>> + if (!backend->reserve && value) {
>> + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
>> + return;
>> + }
>> +
>> if (!host_memory_backend_mr_inited(backend)) {
>> backend->prealloc = value;
>> return;
>> @@ -268,6 +273,7 @@ static void host_memory_backend_init(Object *obj)
>> /* TODO: convert access to globals to compat properties */
>> backend->merge = machine_mem_merge(machine);
>> backend->dump = machine_dump_guest_core(machine);
>> + backend->reserve = true;
>> backend->prealloc_threads = 1;
>> }
>>
>> @@ -426,6 +432,28 @@ static void host_memory_backend_set_share(Object *o, bool value, Error **errp)
>> backend->share = value;
>> }
>>
>> +static bool host_memory_backend_get_reserve(Object *o, Error **errp)
>> +{
>> + HostMemoryBackend *backend = MEMORY_BACKEND(o);
>> +
>> + return backend->reserve;
>> +}
>> +
>> +static void host_memory_backend_set_reserve(Object *o, bool value, Error **errp)
>> +{
>> + HostMemoryBackend *backend = MEMORY_BACKEND(o);
>> +
>> + if (host_memory_backend_mr_inited(backend)) {
>> + error_setg(errp, "cannot change property value");
>> + return;
>> + }
>> + if (backend->prealloc && !value) {
>> + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
>> + return;
>> + }
>> + backend->reserve = value;
>> +}
>> +
>> static bool
>> host_memory_backend_get_use_canonical_path(Object *obj, Error **errp)
>> {
>> @@ -494,6 +522,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
>> host_memory_backend_get_share, host_memory_backend_set_share);
>> object_class_property_set_description(oc, "share",
>> "Mark the memory as private to QEMU or shared");
>> + object_class_property_add_bool(oc, "reserve",
>> + host_memory_backend_get_reserve, host_memory_backend_set_reserve);
>> + object_class_property_set_description(oc, "reserve",
>> + "Reserve swap space (or huge pages) if applicable");
>> /*
>> * Do not delete/rename option. This option must be considered stable
>> * (as if it didn't have the 'x-' prefix including deprecation period) as
>> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
>> index df5644723a..9ff5c16963 100644
>> --- a/include/sysemu/hostmem.h
>> +++ b/include/sysemu/hostmem.h
>> @@ -64,7 +64,7 @@ struct HostMemoryBackend {
>> /* protected */
>> uint64_t size;
>> bool merge, dump, use_canonical_path;
>> - bool prealloc, is_mapped, share;
>> + bool prealloc, is_mapped, share, reserve;
>> uint32_t prealloc_threads;
>> DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
>> HostMemPolicy policy;
>> diff --git a/qapi/qom.json b/qapi/qom.json
>> index cd0e76d564..4fa3137aab 100644
>> --- a/qapi/qom.json
>> +++ b/qapi/qom.json
>> @@ -545,6 +545,9 @@
>> # @share: if false, the memory is private to QEMU; if true, it is shared
>> # (default: false)
>> #
>> +# @reserve: if true, reserve swap space (or huge pages) if applicable
>> +# default: true)
>> +#
>> # @size: size of the memory region in bytes
>> #
>> # @x-use-canonical-path-for-ramblock-id: if true, the canoncial path is used
>> @@ -556,6 +559,12 @@
>> # false generally, but true for machine
>> # types <= 4.0)
>> #
>> +# Note: prealloc=true and reserve=false cannot be set at the same time. With
>> +# reserve=true, the behavior depends on the operating system: for example,
>> +# Linux will not reserve swap space for shared file mappings --
>> +# "not applicable". In contrast, reserve=false will bail out if it cannot
>> +# be configured accordingly.
>> +#
>> # Since: 2.1
>> ##
>> { 'struct': 'MemoryBackendProperties',
>> @@ -566,6 +575,7 @@
>> '*prealloc': 'bool',
>> '*prealloc-threads': 'uint32',
>> '*share': 'bool',
>> + '*reserve': 'bool',
>> 'size': 'size',
>> '*x-use-canonical-path-for-ramblock-id': 'bool' } }
>
> IIUC from the previous patch in the series, 'reserve' is only implemented
> on Linux. If we make this QAPI prop dependant on CONFIG_LINUX, then mgmt
> apps will do the right thing to detect what platform(s) it works on.
Would that mean only conditionally calling (or ifdeffing) the
object_property_add_* in backends/hostmem.c?
Note that the "share" property is basically ignored on Windows and only
implemented on POSIX and we don't even bail out when set ...
Thanks!
--
Thanks,
David / dhildenb
On Tue, May 04, 2021 at 01:08:02PM +0200, David Hildenbrand wrote:
> On 04.05.21 12:12, Daniel P. Berrangé wrote:
> > On Wed, Apr 28, 2021 at 03:37:49PM +0200, David Hildenbrand wrote:
> > > Let's provide a way to control the use of RAM_NORESERVE via memory
> > > backends using the "reserve" property which defaults to true (old
> > > behavior).
> > >
> > > Only Linux currently supports clearing the flag (and support is checked at
> > > runtime, depending on the setting of "/proc/sys/vm/overcommit_memory").
> > > Windows and other POSIX systems will bail out with "reserve=false".
> > >
> > > The target use case is virtio-mem, which dynamically exposes memory
> > > inside a large, sparse memory area to the VM. This essentially allows
> > > avoiding to set "/proc/sys/vm/overcommit_memory == 0") when using
> > > virtio-mem and also supporting hugetlbfs in the future.
> > >
> > > Reviewed-by: Peter Xu <peterx@redhat.com>
> > > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > > Reviewed-by: Markus Armbruster <armbru@redhat.com>
> > > Acked-by: Eduardo Habkost <ehabkost@redhat.com> for memory backend and machine core
> > > Cc: Markus Armbruster <armbru@redhat.com>
> > > Cc: Eric Blake <eblake@redhat.com>
> > > Cc: Igor Mammedov <imammedo@redhat.com>
> > > Signed-off-by: David Hildenbrand <david@redhat.com>
> > > ---
> > > backends/hostmem-file.c | 11 ++++++-----
> > > backends/hostmem-memfd.c | 1 +
> > > backends/hostmem-ram.c | 1 +
> > > backends/hostmem.c | 32 ++++++++++++++++++++++++++++++++
> > > include/sysemu/hostmem.h | 2 +-
> > > qapi/qom.json | 10 ++++++++++
> > > 6 files changed, 51 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> > > index b683da9daf..9d550e53d4 100644
> > > --- a/backends/hostmem-file.c
> > > +++ b/backends/hostmem-file.c
> > > @@ -40,6 +40,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > > object_get_typename(OBJECT(backend)));
> > > #else
> > > HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
> > > + uint32_t ram_flags;
> > > gchar *name;
> > > if (!backend->size) {
> > > @@ -52,11 +53,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > > }
> > > name = host_memory_backend_get_name(backend);
> > > - memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
> > > - name,
> > > - backend->size, fb->align,
> > > - (backend->share ? RAM_SHARED : 0) |
> > > - (fb->is_pmem ? RAM_PMEM : 0),
> > > + ram_flags = backend->share ? RAM_SHARED : 0;
> > > + ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
> > > + ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
> > > + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
> > > + backend->size, fb->align, ram_flags,
> > > fb->mem_path, fb->readonly, errp);
> > > g_free(name);
> > > #endif
> > > diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
> > > index 93b5d1a4cf..f3436b623d 100644
> > > --- a/backends/hostmem-memfd.c
> > > +++ b/backends/hostmem-memfd.c
> > > @@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > > 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);
> > > g_free(name);
> > > diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
> > > index 741e701062..b8e55cdbd0 100644
> > > --- a/backends/hostmem-ram.c
> > > +++ b/backends/hostmem-ram.c
> > > @@ -29,6 +29,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > > 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);
> > > g_free(name);
> > > diff --git a/backends/hostmem.c b/backends/hostmem.c
> > > index c6c1ff5b99..58fdc1b658 100644
> > > --- a/backends/hostmem.c
> > > +++ b/backends/hostmem.c
> > > @@ -217,6 +217,11 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
> > > Error *local_err = NULL;
> > > HostMemoryBackend *backend = MEMORY_BACKEND(obj);
> > > + if (!backend->reserve && value) {
> > > + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
> > > + return;
> > > + }
> > > +
> > > if (!host_memory_backend_mr_inited(backend)) {
> > > backend->prealloc = value;
> > > return;
> > > @@ -268,6 +273,7 @@ static void host_memory_backend_init(Object *obj)
> > > /* TODO: convert access to globals to compat properties */
> > > backend->merge = machine_mem_merge(machine);
> > > backend->dump = machine_dump_guest_core(machine);
> > > + backend->reserve = true;
> > > backend->prealloc_threads = 1;
> > > }
> > > @@ -426,6 +432,28 @@ static void host_memory_backend_set_share(Object *o, bool value, Error **errp)
> > > backend->share = value;
> > > }
> > > +static bool host_memory_backend_get_reserve(Object *o, Error **errp)
> > > +{
> > > + HostMemoryBackend *backend = MEMORY_BACKEND(o);
> > > +
> > > + return backend->reserve;
> > > +}
> > > +
> > > +static void host_memory_backend_set_reserve(Object *o, bool value, Error **errp)
> > > +{
> > > + HostMemoryBackend *backend = MEMORY_BACKEND(o);
> > > +
> > > + if (host_memory_backend_mr_inited(backend)) {
> > > + error_setg(errp, "cannot change property value");
> > > + return;
> > > + }
> > > + if (backend->prealloc && !value) {
> > > + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
> > > + return;
> > > + }
> > > + backend->reserve = value;
> > > +}
> > > +
> > > static bool
> > > host_memory_backend_get_use_canonical_path(Object *obj, Error **errp)
> > > {
> > > @@ -494,6 +522,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
> > > host_memory_backend_get_share, host_memory_backend_set_share);
> > > object_class_property_set_description(oc, "share",
> > > "Mark the memory as private to QEMU or shared");
> > > + object_class_property_add_bool(oc, "reserve",
> > > + host_memory_backend_get_reserve, host_memory_backend_set_reserve);
> > > + object_class_property_set_description(oc, "reserve",
> > > + "Reserve swap space (or huge pages) if applicable");
> > > /*
> > > * Do not delete/rename option. This option must be considered stable
> > > * (as if it didn't have the 'x-' prefix including deprecation period) as
> > > diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
> > > index df5644723a..9ff5c16963 100644
> > > --- a/include/sysemu/hostmem.h
> > > +++ b/include/sysemu/hostmem.h
> > > @@ -64,7 +64,7 @@ struct HostMemoryBackend {
> > > /* protected */
> > > uint64_t size;
> > > bool merge, dump, use_canonical_path;
> > > - bool prealloc, is_mapped, share;
> > > + bool prealloc, is_mapped, share, reserve;
> > > uint32_t prealloc_threads;
> > > DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
> > > HostMemPolicy policy;
> > > diff --git a/qapi/qom.json b/qapi/qom.json
> > > index cd0e76d564..4fa3137aab 100644
> > > --- a/qapi/qom.json
> > > +++ b/qapi/qom.json
> > > @@ -545,6 +545,9 @@
> > > # @share: if false, the memory is private to QEMU; if true, it is shared
> > > # (default: false)
> > > #
> > > +# @reserve: if true, reserve swap space (or huge pages) if applicable
> > > +# default: true)
> > > +#
> > > # @size: size of the memory region in bytes
> > > #
> > > # @x-use-canonical-path-for-ramblock-id: if true, the canoncial path is used
> > > @@ -556,6 +559,12 @@
> > > # false generally, but true for machine
> > > # types <= 4.0)
> > > #
> > > +# Note: prealloc=true and reserve=false cannot be set at the same time. With
> > > +# reserve=true, the behavior depends on the operating system: for example,
> > > +# Linux will not reserve swap space for shared file mappings --
> > > +# "not applicable". In contrast, reserve=false will bail out if it cannot
> > > +# be configured accordingly.
> > > +#
> > > # Since: 2.1
> > > ##
> > > { 'struct': 'MemoryBackendProperties',
> > > @@ -566,6 +575,7 @@
> > > '*prealloc': 'bool',
> > > '*prealloc-threads': 'uint32',
> > > '*share': 'bool',
> > > + '*reserve': 'bool',
> > > 'size': 'size',
> > > '*x-use-canonical-path-for-ramblock-id': 'bool' } }
> >
> > IIUC from the previous patch in the series, 'reserve' is only implemented
> > on Linux. If we make this QAPI prop dependant on CONFIG_LINUX, then mgmt
> > apps will do the right thing to detect what platform(s) it works on.
>
> Would that mean only conditionally calling (or ifdeffing) the
> object_property_add_* in backends/hostmem.c?
Yes, any code which refers to the property would need to have
matching conditionals. The plus side is that you don't have to
do error reporting to say it doesn't exist, because it becomes
impossible for the mgmt app to even set the property in the
first place on platforms where it doesn't exist.
> Note that the "share" property is basically ignored on Windows and only
> implemented on POSIX and we don't even bail out when set ...
It might pre-date the support for conditionals in QAPI. Silently ignoring
things that can't be supported is even worse !
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 04.05.21 13:18, Daniel P. Berrangé wrote:
> On Tue, May 04, 2021 at 01:08:02PM +0200, David Hildenbrand wrote:
>> On 04.05.21 12:12, Daniel P. Berrangé wrote:
>>> On Wed, Apr 28, 2021 at 03:37:49PM +0200, David Hildenbrand wrote:
>>>> Let's provide a way to control the use of RAM_NORESERVE via memory
>>>> backends using the "reserve" property which defaults to true (old
>>>> behavior).
>>>>
>>>> Only Linux currently supports clearing the flag (and support is checked at
>>>> runtime, depending on the setting of "/proc/sys/vm/overcommit_memory").
>>>> Windows and other POSIX systems will bail out with "reserve=false".
>>>>
>>>> The target use case is virtio-mem, which dynamically exposes memory
>>>> inside a large, sparse memory area to the VM. This essentially allows
>>>> avoiding to set "/proc/sys/vm/overcommit_memory == 0") when using
>>>> virtio-mem and also supporting hugetlbfs in the future.
>>>>
>>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>>> Acked-by: Eduardo Habkost <ehabkost@redhat.com> for memory backend and machine core
>>>> Cc: Markus Armbruster <armbru@redhat.com>
>>>> Cc: Eric Blake <eblake@redhat.com>
>>>> Cc: Igor Mammedov <imammedo@redhat.com>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>> backends/hostmem-file.c | 11 ++++++-----
>>>> backends/hostmem-memfd.c | 1 +
>>>> backends/hostmem-ram.c | 1 +
>>>> backends/hostmem.c | 32 ++++++++++++++++++++++++++++++++
>>>> include/sysemu/hostmem.h | 2 +-
>>>> qapi/qom.json | 10 ++++++++++
>>>> 6 files changed, 51 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
>>>> index b683da9daf..9d550e53d4 100644
>>>> --- a/backends/hostmem-file.c
>>>> +++ b/backends/hostmem-file.c
>>>> @@ -40,6 +40,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>>> object_get_typename(OBJECT(backend)));
>>>> #else
>>>> HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
>>>> + uint32_t ram_flags;
>>>> gchar *name;
>>>> if (!backend->size) {
>>>> @@ -52,11 +53,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>>> }
>>>> name = host_memory_backend_get_name(backend);
>>>> - memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>>>> - name,
>>>> - backend->size, fb->align,
>>>> - (backend->share ? RAM_SHARED : 0) |
>>>> - (fb->is_pmem ? RAM_PMEM : 0),
>>>> + ram_flags = backend->share ? RAM_SHARED : 0;
>>>> + ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
>>>> + ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
>>>> + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
>>>> + backend->size, fb->align, ram_flags,
>>>> fb->mem_path, fb->readonly, errp);
>>>> g_free(name);
>>>> #endif
>>>> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
>>>> index 93b5d1a4cf..f3436b623d 100644
>>>> --- a/backends/hostmem-memfd.c
>>>> +++ b/backends/hostmem-memfd.c
>>>> @@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>>> 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);
>>>> g_free(name);
>>>> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
>>>> index 741e701062..b8e55cdbd0 100644
>>>> --- a/backends/hostmem-ram.c
>>>> +++ b/backends/hostmem-ram.c
>>>> @@ -29,6 +29,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>>> 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);
>>>> g_free(name);
>>>> diff --git a/backends/hostmem.c b/backends/hostmem.c
>>>> index c6c1ff5b99..58fdc1b658 100644
>>>> --- a/backends/hostmem.c
>>>> +++ b/backends/hostmem.c
>>>> @@ -217,6 +217,11 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
>>>> Error *local_err = NULL;
>>>> HostMemoryBackend *backend = MEMORY_BACKEND(obj);
>>>> + if (!backend->reserve && value) {
>>>> + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
>>>> + return;
>>>> + }
>>>> +
>>>> if (!host_memory_backend_mr_inited(backend)) {
>>>> backend->prealloc = value;
>>>> return;
>>>> @@ -268,6 +273,7 @@ static void host_memory_backend_init(Object *obj)
>>>> /* TODO: convert access to globals to compat properties */
>>>> backend->merge = machine_mem_merge(machine);
>>>> backend->dump = machine_dump_guest_core(machine);
>>>> + backend->reserve = true;
>>>> backend->prealloc_threads = 1;
>>>> }
>>>> @@ -426,6 +432,28 @@ static void host_memory_backend_set_share(Object *o, bool value, Error **errp)
>>>> backend->share = value;
>>>> }
>>>> +static bool host_memory_backend_get_reserve(Object *o, Error **errp)
>>>> +{
>>>> + HostMemoryBackend *backend = MEMORY_BACKEND(o);
>>>> +
>>>> + return backend->reserve;
>>>> +}
>>>> +
>>>> +static void host_memory_backend_set_reserve(Object *o, bool value, Error **errp)
>>>> +{
>>>> + HostMemoryBackend *backend = MEMORY_BACKEND(o);
>>>> +
>>>> + if (host_memory_backend_mr_inited(backend)) {
>>>> + error_setg(errp, "cannot change property value");
>>>> + return;
>>>> + }
>>>> + if (backend->prealloc && !value) {
>>>> + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
>>>> + return;
>>>> + }
>>>> + backend->reserve = value;
>>>> +}
>>>> +
>>>> static bool
>>>> host_memory_backend_get_use_canonical_path(Object *obj, Error **errp)
>>>> {
>>>> @@ -494,6 +522,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
>>>> host_memory_backend_get_share, host_memory_backend_set_share);
>>>> object_class_property_set_description(oc, "share",
>>>> "Mark the memory as private to QEMU or shared");
>>>> + object_class_property_add_bool(oc, "reserve",
>>>> + host_memory_backend_get_reserve, host_memory_backend_set_reserve);
>>>> + object_class_property_set_description(oc, "reserve",
>>>> + "Reserve swap space (or huge pages) if applicable");
>>>> /*
>>>> * Do not delete/rename option. This option must be considered stable
>>>> * (as if it didn't have the 'x-' prefix including deprecation period) as
>>>> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
>>>> index df5644723a..9ff5c16963 100644
>>>> --- a/include/sysemu/hostmem.h
>>>> +++ b/include/sysemu/hostmem.h
>>>> @@ -64,7 +64,7 @@ struct HostMemoryBackend {
>>>> /* protected */
>>>> uint64_t size;
>>>> bool merge, dump, use_canonical_path;
>>>> - bool prealloc, is_mapped, share;
>>>> + bool prealloc, is_mapped, share, reserve;
>>>> uint32_t prealloc_threads;
>>>> DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
>>>> HostMemPolicy policy;
>>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>>> index cd0e76d564..4fa3137aab 100644
>>>> --- a/qapi/qom.json
>>>> +++ b/qapi/qom.json
>>>> @@ -545,6 +545,9 @@
>>>> # @share: if false, the memory is private to QEMU; if true, it is shared
>>>> # (default: false)
>>>> #
>>>> +# @reserve: if true, reserve swap space (or huge pages) if applicable
>>>> +# default: true)
>>>> +#
>>>> # @size: size of the memory region in bytes
>>>> #
>>>> # @x-use-canonical-path-for-ramblock-id: if true, the canoncial path is used
>>>> @@ -556,6 +559,12 @@
>>>> # false generally, but true for machine
>>>> # types <= 4.0)
>>>> #
>>>> +# Note: prealloc=true and reserve=false cannot be set at the same time. With
>>>> +# reserve=true, the behavior depends on the operating system: for example,
>>>> +# Linux will not reserve swap space for shared file mappings --
>>>> +# "not applicable". In contrast, reserve=false will bail out if it cannot
>>>> +# be configured accordingly.
>>>> +#
>>>> # Since: 2.1
>>>> ##
>>>> { 'struct': 'MemoryBackendProperties',
>>>> @@ -566,6 +575,7 @@
>>>> '*prealloc': 'bool',
>>>> '*prealloc-threads': 'uint32',
>>>> '*share': 'bool',
>>>> + '*reserve': 'bool',
>>>> 'size': 'size',
>>>> '*x-use-canonical-path-for-ramblock-id': 'bool' } }
>>>
>>> IIUC from the previous patch in the series, 'reserve' is only implemented
>>> on Linux. If we make this QAPI prop dependant on CONFIG_LINUX, then mgmt
>>> apps will do the right thing to detect what platform(s) it works on.
>>
>> Would that mean only conditionally calling (or ifdeffing) the
>> object_property_add_* in backends/hostmem.c?
>
> Yes, any code which refers to the property would need to have
> matching conditionals. The plus side is that you don't have to
> do error reporting to say it doesn't exist, because it becomes
> impossible for the mgmt app to even set the property in the
> first place on platforms where it doesn't exist.
It's a little more complicated on Linux, see patch #9. We'll still need
error reporting for some memory backends when the user cofigured
"/proc/sys/vm/overcommit_memory = 2". So we cannot completely get rid of
error reporting I'm afraid.
Which raises the question if registering the propert conditionally just
to handle !Linux slightly better is worth the effort.
(in theory, we might see support for some other POSIX systems in the
future; Windows is more tricky)
--
Thanks,
David / dhildenb
On 04.05.21 13:18, Daniel P. Berrangé wrote:
> On Tue, May 04, 2021 at 01:08:02PM +0200, David Hildenbrand wrote:
>> On 04.05.21 12:12, Daniel P. Berrangé wrote:
>>> On Wed, Apr 28, 2021 at 03:37:49PM +0200, David Hildenbrand wrote:
>>>> Let's provide a way to control the use of RAM_NORESERVE via memory
>>>> backends using the "reserve" property which defaults to true (old
>>>> behavior).
>>>>
>>>> Only Linux currently supports clearing the flag (and support is checked at
>>>> runtime, depending on the setting of "/proc/sys/vm/overcommit_memory").
>>>> Windows and other POSIX systems will bail out with "reserve=false".
>>>>
>>>> The target use case is virtio-mem, which dynamically exposes memory
>>>> inside a large, sparse memory area to the VM. This essentially allows
>>>> avoiding to set "/proc/sys/vm/overcommit_memory == 0") when using
>>>> virtio-mem and also supporting hugetlbfs in the future.
>>>>
>>>> Reviewed-by: Peter Xu <peterx@redhat.com>
>>>> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
>>>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>>>> Acked-by: Eduardo Habkost <ehabkost@redhat.com> for memory backend and machine core
>>>> Cc: Markus Armbruster <armbru@redhat.com>
>>>> Cc: Eric Blake <eblake@redhat.com>
>>>> Cc: Igor Mammedov <imammedo@redhat.com>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> ---
>>>> backends/hostmem-file.c | 11 ++++++-----
>>>> backends/hostmem-memfd.c | 1 +
>>>> backends/hostmem-ram.c | 1 +
>>>> backends/hostmem.c | 32 ++++++++++++++++++++++++++++++++
>>>> include/sysemu/hostmem.h | 2 +-
>>>> qapi/qom.json | 10 ++++++++++
>>>> 6 files changed, 51 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
>>>> index b683da9daf..9d550e53d4 100644
>>>> --- a/backends/hostmem-file.c
>>>> +++ b/backends/hostmem-file.c
>>>> @@ -40,6 +40,7 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>>> object_get_typename(OBJECT(backend)));
>>>> #else
>>>> HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
>>>> + uint32_t ram_flags;
>>>> gchar *name;
>>>> if (!backend->size) {
>>>> @@ -52,11 +53,11 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>>> }
>>>> name = host_memory_backend_get_name(backend);
>>>> - memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>>>> - name,
>>>> - backend->size, fb->align,
>>>> - (backend->share ? RAM_SHARED : 0) |
>>>> - (fb->is_pmem ? RAM_PMEM : 0),
>>>> + ram_flags = backend->share ? RAM_SHARED : 0;
>>>> + ram_flags |= backend->reserve ? 0 : RAM_NORESERVE;
>>>> + ram_flags |= fb->is_pmem ? RAM_PMEM : 0;
>>>> + memory_region_init_ram_from_file(&backend->mr, OBJECT(backend), name,
>>>> + backend->size, fb->align, ram_flags,
>>>> fb->mem_path, fb->readonly, errp);
>>>> g_free(name);
>>>> #endif
>>>> diff --git a/backends/hostmem-memfd.c b/backends/hostmem-memfd.c
>>>> index 93b5d1a4cf..f3436b623d 100644
>>>> --- a/backends/hostmem-memfd.c
>>>> +++ b/backends/hostmem-memfd.c
>>>> @@ -55,6 +55,7 @@ memfd_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>>> 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);
>>>> g_free(name);
>>>> diff --git a/backends/hostmem-ram.c b/backends/hostmem-ram.c
>>>> index 741e701062..b8e55cdbd0 100644
>>>> --- a/backends/hostmem-ram.c
>>>> +++ b/backends/hostmem-ram.c
>>>> @@ -29,6 +29,7 @@ ram_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
>>>> 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);
>>>> g_free(name);
>>>> diff --git a/backends/hostmem.c b/backends/hostmem.c
>>>> index c6c1ff5b99..58fdc1b658 100644
>>>> --- a/backends/hostmem.c
>>>> +++ b/backends/hostmem.c
>>>> @@ -217,6 +217,11 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
>>>> Error *local_err = NULL;
>>>> HostMemoryBackend *backend = MEMORY_BACKEND(obj);
>>>> + if (!backend->reserve && value) {
>>>> + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
>>>> + return;
>>>> + }
>>>> +
>>>> if (!host_memory_backend_mr_inited(backend)) {
>>>> backend->prealloc = value;
>>>> return;
>>>> @@ -268,6 +273,7 @@ static void host_memory_backend_init(Object *obj)
>>>> /* TODO: convert access to globals to compat properties */
>>>> backend->merge = machine_mem_merge(machine);
>>>> backend->dump = machine_dump_guest_core(machine);
>>>> + backend->reserve = true;
>>>> backend->prealloc_threads = 1;
>>>> }
>>>> @@ -426,6 +432,28 @@ static void host_memory_backend_set_share(Object *o, bool value, Error **errp)
>>>> backend->share = value;
>>>> }
>>>> +static bool host_memory_backend_get_reserve(Object *o, Error **errp)
>>>> +{
>>>> + HostMemoryBackend *backend = MEMORY_BACKEND(o);
>>>> +
>>>> + return backend->reserve;
>>>> +}
>>>> +
>>>> +static void host_memory_backend_set_reserve(Object *o, bool value, Error **errp)
>>>> +{
>>>> + HostMemoryBackend *backend = MEMORY_BACKEND(o);
>>>> +
>>>> + if (host_memory_backend_mr_inited(backend)) {
>>>> + error_setg(errp, "cannot change property value");
>>>> + return;
>>>> + }
>>>> + if (backend->prealloc && !value) {
>>>> + error_setg(errp, "'prealloc=on' and 'reserve=off' are incompatible");
>>>> + return;
>>>> + }
>>>> + backend->reserve = value;
>>>> +}
>>>> +
>>>> static bool
>>>> host_memory_backend_get_use_canonical_path(Object *obj, Error **errp)
>>>> {
>>>> @@ -494,6 +522,10 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
>>>> host_memory_backend_get_share, host_memory_backend_set_share);
>>>> object_class_property_set_description(oc, "share",
>>>> "Mark the memory as private to QEMU or shared");
>>>> + object_class_property_add_bool(oc, "reserve",
>>>> + host_memory_backend_get_reserve, host_memory_backend_set_reserve);
>>>> + object_class_property_set_description(oc, "reserve",
>>>> + "Reserve swap space (or huge pages) if applicable");
>>>> /*
>>>> * Do not delete/rename option. This option must be considered stable
>>>> * (as if it didn't have the 'x-' prefix including deprecation period) as
>>>> diff --git a/include/sysemu/hostmem.h b/include/sysemu/hostmem.h
>>>> index df5644723a..9ff5c16963 100644
>>>> --- a/include/sysemu/hostmem.h
>>>> +++ b/include/sysemu/hostmem.h
>>>> @@ -64,7 +64,7 @@ struct HostMemoryBackend {
>>>> /* protected */
>>>> uint64_t size;
>>>> bool merge, dump, use_canonical_path;
>>>> - bool prealloc, is_mapped, share;
>>>> + bool prealloc, is_mapped, share, reserve;
>>>> uint32_t prealloc_threads;
>>>> DECLARE_BITMAP(host_nodes, MAX_NODES + 1);
>>>> HostMemPolicy policy;
>>>> diff --git a/qapi/qom.json b/qapi/qom.json
>>>> index cd0e76d564..4fa3137aab 100644
>>>> --- a/qapi/qom.json
>>>> +++ b/qapi/qom.json
>>>> @@ -545,6 +545,9 @@
>>>> # @share: if false, the memory is private to QEMU; if true, it is shared
>>>> # (default: false)
>>>> #
>>>> +# @reserve: if true, reserve swap space (or huge pages) if applicable
>>>> +# default: true)
>>>> +#
>>>> # @size: size of the memory region in bytes
>>>> #
>>>> # @x-use-canonical-path-for-ramblock-id: if true, the canoncial path is used
>>>> @@ -556,6 +559,12 @@
>>>> # false generally, but true for machine
>>>> # types <= 4.0)
>>>> #
>>>> +# Note: prealloc=true and reserve=false cannot be set at the same time. With
>>>> +# reserve=true, the behavior depends on the operating system: for example,
>>>> +# Linux will not reserve swap space for shared file mappings --
>>>> +# "not applicable". In contrast, reserve=false will bail out if it cannot
>>>> +# be configured accordingly.
>>>> +#
>>>> # Since: 2.1
>>>> ##
>>>> { 'struct': 'MemoryBackendProperties',
>>>> @@ -566,6 +575,7 @@
>>>> '*prealloc': 'bool',
>>>> '*prealloc-threads': 'uint32',
>>>> '*share': 'bool',
>>>> + '*reserve': 'bool',
>>>> 'size': 'size',
>>>> '*x-use-canonical-path-for-ramblock-id': 'bool' } }
>>>
>>> IIUC from the previous patch in the series, 'reserve' is only implemented
>>> on Linux. If we make this QAPI prop dependant on CONFIG_LINUX, then mgmt
>>> apps will do the right thing to detect what platform(s) it works on.
>>
>> Would that mean only conditionally calling (or ifdeffing) the
>> object_property_add_* in backends/hostmem.c?
>
> Yes, any code which refers to the property would need to have
> matching conditionals. The plus side is that you don't have to
> do error reporting to say it doesn't exist, because it becomes
> impossible for the mgmt app to even set the property in the
> first place on platforms where it doesn't exist.
I could fold in the following change, which should be sufficient.
In that case, setting the property to "false" could only fail if
the user messes with the memory overcommit configuration in /sys/.
diff --git a/backends/hostmem.c b/backends/hostmem.c
index e5038e9cab..4c05862ed5 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -431,6 +431,7 @@ static void host_memory_backend_set_share(Object *o, bool value, Error **errp)
backend->share = value;
}
+#ifdef CONFIG_LINUX
static bool host_memory_backend_get_reserve(Object *o, Error **errp)
{
HostMemoryBackend *backend = MEMORY_BACKEND(o);
@@ -452,6 +453,7 @@ static void host_memory_backend_set_reserve(Object *o, bool value, Error **errp)
}
backend->reserve = value;
}
+#endif /* CONFIG_LINUX */
static bool
host_memory_backend_get_use_canonical_path(Object *obj, Error **errp)
@@ -521,10 +523,12 @@ host_memory_backend_class_init(ObjectClass *oc, void *data)
host_memory_backend_get_share, host_memory_backend_set_share);
object_class_property_set_description(oc, "share",
"Mark the memory as private to QEMU or shared");
+#ifdef CONFIG_LINUX
object_class_property_add_bool(oc, "reserve",
host_memory_backend_get_reserve, host_memory_backend_set_reserve);
object_class_property_set_description(oc, "reserve",
"Reserve swap space (or huge pages) if applicable");
+#endif /* CONFIG_LINUX */
/*
* Do not delete/rename option. This option must be considered stable
* (as if it didn't have the 'x-' prefix including deprecation period) as
diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index 9bedc77bb4..76b22b00d6 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -112,8 +112,10 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
m->value->prealloc ? "true" : "false");
monitor_printf(mon, " share: %s\n",
m->value->share ? "true" : "false");
- monitor_printf(mon, " reserve: %s\n",
- m->value->reserve ? "true" : "false");
+ if (m->value->has_reserve) {
+ monitor_printf(mon, " reserve: %s\n",
+ m->value->reserve ? "true" : "false");
+ }
monitor_printf(mon, " policy: %s\n",
HostMemPolicy_str(m->value->policy));
visit_complete(v, &str);
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 773904dbca..8922cc511f 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -173,7 +173,10 @@ static int query_memdev(Object *obj, void *opaque)
m->dump = object_property_get_bool(obj, "dump", &error_abort);
m->prealloc = object_property_get_bool(obj, "prealloc", &error_abort);
m->share = object_property_get_bool(obj, "share", &error_abort);
+#ifdef CONFIG_LINUX
m->reserve = object_property_get_bool(obj, "reserve", &error_abort);
+ m->has_reserve = true;
+#endif /* CONFIG_LINUX */
m->policy = object_property_get_enum(obj, "policy", "HostMemPolicy",
&error_abort);
host_nodes = object_property_get_qobject(obj,
diff --git a/qapi/machine.json b/qapi/machine.json
index ea68f1a083..1cfb16e6eb 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -818,7 +818,7 @@
'dump': 'bool',
'prealloc': 'bool',
'share': 'bool',
- 'reserve': 'bool',
+ '*reserve': 'bool',
'host-nodes': ['uint16'],
'policy': 'HostMemPolicy' }}
--
2.30.2
Thanks!
--
Thanks,
David / dhildenb
© 2016 - 2026 Red Hat, Inc.