|
From: | John Snow |
Subject: | Re: [PATCH RESEND] hostmem: Don't report pmem attribute if unsupported |
Date: | Tue, 16 Feb 2021 18:07:39 -0500 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 |
On 2/16/21 5:23 PM, Eduardo Habkost wrote:
On Tue, Jan 26, 2021 at 08:48:25AM +0100, 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> Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> Tested-by: Daniel Henrique Barboza <danielhb413@gmail.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com>I'm not a fan of making availability of properties conditional (even if at compile time), but if this helps libvirt I guess it makes sense.
Compile time might be OK, but if we want to describe everything via QAPI eventually, we just need to be able to describe that compile-time requisite appropriately.
Conditional at run-time is I think the thing that absolutely has to go wherever it surfaces.
CCing John, who has been thinking a lot about these questions.
Thanks for the heads up. Good reminder that libvirt uses the existence of properties as a bellwether for feature support. I don't think I like that idea, but I like breaking libvirt even less.
--js
I'm queueing this on machine-next. Thanks, and sorry for the delay!--- This is just a resend of a patch I've sent earlier with Reviewed-by and Tested-by added: https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg04558.html 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_LIBPMEMstatic 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
[Prev in Thread] | Current Thread | [Next in Thread] |