* [PATCH] hostmem: Don't report pmem attribute if unsupported
@ 2021-01-19 12:35 Michal Privoznik
2021-01-19 16:55 ` Daniel Henrique Barboza
2021-01-20 10:07 ` Igor Mammedov
0 siblings, 2 replies; 3+ messages in thread
From: Michal Privoznik @ 2021-01-19 12:35 UTC (permalink / raw)
To: qemu-devel
When management applications (like Libvirt) want to check whether
memory-backend-file.pmem is supported they can list object
properties using 'qom-list-properties'. However, 'pmem' is
declared always (and thus reported always) and only at runtime
QEMU errors out if it was built without libpmem (and thus can not
guarantee write persistence). This is suboptimal since we have
ability to declare attributes at compile time.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1915216
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
backends/hostmem-file.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index 40e1e5b3e3..7e30eb5985 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -123,6 +123,7 @@ static void file_memory_backend_set_align(Object *o, Visitor *v,
fb->align = val;
}
+#ifdef CONFIG_LIBPMEM
static bool file_memory_backend_get_pmem(Object *o, Error **errp)
{
return MEMORY_BACKEND_FILE(o)->is_pmem;
@@ -139,17 +140,9 @@ static void file_memory_backend_set_pmem(Object *o, bool value, Error **errp)
return;
}
-#ifndef CONFIG_LIBPMEM
- if (value) {
- error_setg(errp, "Lack of libpmem support while setting the 'pmem=on'"
- " of %s. We can't ensure data persistence.",
- object_get_typename(o));
- return;
- }
-#endif
-
fb->is_pmem = value;
}
+#endif /* CONFIG_LIBPMEM */
static void file_backend_unparent(Object *obj)
{
@@ -180,8 +173,10 @@ file_backend_class_init(ObjectClass *oc, void *data)
file_memory_backend_get_align,
file_memory_backend_set_align,
NULL, NULL);
+#ifdef CONFIG_LIBPMEM
object_class_property_add_bool(oc, "pmem",
file_memory_backend_get_pmem, file_memory_backend_set_pmem);
+#endif
}
static void file_backend_instance_finalize(Object *o)
--
2.26.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] hostmem: Don't report pmem attribute if unsupported
2021-01-19 12:35 [PATCH] hostmem: Don't report pmem attribute if unsupported Michal Privoznik
@ 2021-01-19 16:55 ` Daniel Henrique Barboza
2021-01-20 10:07 ` Igor Mammedov
1 sibling, 0 replies; 3+ messages in thread
From: Daniel Henrique Barboza @ 2021-01-19 16:55 UTC (permalink / raw)
To: Michal Privoznik, qemu-devel
On 1/19/21 9:35 AM, Michal Privoznik wrote:
> When management applications (like Libvirt) want to check whether
> memory-backend-file.pmem is supported they can list object
> properties using 'qom-list-properties'. However, 'pmem' is
> declared always (and thus reported always) and only at runtime
> QEMU errors out if it was built without libpmem (and thus can not
> guarantee write persistence). This is suboptimal since we have
> ability to declare attributes at compile time.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1915216
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
Testing it on a host that doesn't have libpmem support:
- without this patch:
{"execute": "qom-list-properties", "arguments": {"typename": "memory-backend-file"}}
{"return": [
(...)
{"name": "pmem", "type": "bool"},
(...)
]}
- with this patch there is no 'pmem' support reported:
{"execute": "qom-list-properties", "arguments": {"typename": "memory-backend-file"}}
{"return": [
{"name": "type", "type": "string"},
{"name": "policy", "description": "Set the NUMA policy", "type": "HostMemPolicy"},
{"name": "prealloc-threads", "description": "Number of CPU threads to use for prealloc", "type": "int"},
{"name": "dump", "description": "Set to 'off' to exclude from core dump", "type": "bool"},
{"name": "share", "description": "Mark the memory as private to QEMU or shared", "type": "bool"},
{"name": "prealloc", "description": "Preallocate memory", "type": "bool"},
{"name": "size", "description": "Size of the memory region (ex: 500M)", "type": "int"},
{"name": "x-use-canonical-path-for-ramblock-id", "type": "bool"},
{"name": "host-nodes", "description": "Binds memory to the list of NUMA host nodes", "type": "int"},
{"name": "merge", "description": "Mark memory as mergeable", "type": "bool"},
{"name": "align", "type": "int"},
{"name": "mem-path", "type": "string"},
{"name": "discard-data", "type": "bool"}
]}
Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> backends/hostmem-file.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 40e1e5b3e3..7e30eb5985 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -123,6 +123,7 @@ static void file_memory_backend_set_align(Object *o, Visitor *v,
> fb->align = val;
> }
>
> +#ifdef CONFIG_LIBPMEM
> static bool file_memory_backend_get_pmem(Object *o, Error **errp)
> {
> return MEMORY_BACKEND_FILE(o)->is_pmem;
> @@ -139,17 +140,9 @@ static void file_memory_backend_set_pmem(Object *o, bool value, Error **errp)
> return;
> }
>
> -#ifndef CONFIG_LIBPMEM
> - if (value) {
> - error_setg(errp, "Lack of libpmem support while setting the 'pmem=on'"
> - " of %s. We can't ensure data persistence.",
> - object_get_typename(o));
> - return;
> - }
> -#endif
> -
> fb->is_pmem = value;
> }
> +#endif /* CONFIG_LIBPMEM */
>
> static void file_backend_unparent(Object *obj)
> {
> @@ -180,8 +173,10 @@ file_backend_class_init(ObjectClass *oc, void *data)
> file_memory_backend_get_align,
> file_memory_backend_set_align,
> NULL, NULL);
> +#ifdef CONFIG_LIBPMEM
> object_class_property_add_bool(oc, "pmem",
> file_memory_backend_get_pmem, file_memory_backend_set_pmem);
> +#endif
> }
>
> static void file_backend_instance_finalize(Object *o)
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] hostmem: Don't report pmem attribute if unsupported
2021-01-19 12:35 [PATCH] hostmem: Don't report pmem attribute if unsupported Michal Privoznik
2021-01-19 16:55 ` Daniel Henrique Barboza
@ 2021-01-20 10:07 ` Igor Mammedov
1 sibling, 0 replies; 3+ messages in thread
From: Igor Mammedov @ 2021-01-20 10:07 UTC (permalink / raw)
To: Michal Privoznik; +Cc: qemu-devel
On Tue, 19 Jan 2021 13:35:35 +0100
Michal Privoznik <mprivozn@redhat.com> wrote:
> When management applications (like Libvirt) want to check whether
> memory-backend-file.pmem is supported they can list object
> properties using 'qom-list-properties'. However, 'pmem' is
> declared always (and thus reported always) and only at runtime
> QEMU errors out if it was built without libpmem (and thus can not
> guarantee write persistence). This is suboptimal since we have
> ability to declare attributes at compile time.
>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1915216
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> backends/hostmem-file.c | 13 ++++---------
> 1 file changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> index 40e1e5b3e3..7e30eb5985 100644
> --- a/backends/hostmem-file.c
> +++ b/backends/hostmem-file.c
> @@ -123,6 +123,7 @@ static void file_memory_backend_set_align(Object *o, Visitor *v,
> fb->align = val;
> }
>
> +#ifdef CONFIG_LIBPMEM
> static bool file_memory_backend_get_pmem(Object *o, Error **errp)
> {
> return MEMORY_BACKEND_FILE(o)->is_pmem;
> @@ -139,17 +140,9 @@ static void file_memory_backend_set_pmem(Object *o, bool value, Error **errp)
> return;
> }
>
> -#ifndef CONFIG_LIBPMEM
> - if (value) {
> - error_setg(errp, "Lack of libpmem support while setting the 'pmem=on'"
> - " of %s. We can't ensure data persistence.",
> - object_get_typename(o));
> - return;
> - }
> -#endif
> -
> fb->is_pmem = value;
> }
> +#endif /* CONFIG_LIBPMEM */
>
> static void file_backend_unparent(Object *obj)
> {
> @@ -180,8 +173,10 @@ file_backend_class_init(ObjectClass *oc, void *data)
> file_memory_backend_get_align,
> file_memory_backend_set_align,
> NULL, NULL);
> +#ifdef CONFIG_LIBPMEM
> object_class_property_add_bool(oc, "pmem",
> file_memory_backend_get_pmem, file_memory_backend_set_pmem);
> +#endif
> }
>
> static void file_backend_instance_finalize(Object *o)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-01-20 10:08 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-19 12:35 [PATCH] hostmem: Don't report pmem attribute if unsupported Michal Privoznik
2021-01-19 16:55 ` Daniel Henrique Barboza
2021-01-20 10:07 ` Igor Mammedov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).