Re: [PATCH v6 10/15] hostmem: Wire up RAM_NORESERVE via "reserve" property

From: Markus Armbruster <armbru@redhat.com>
To: David Hildenbrand <david@redhat.com>
Cc: "Marcel Apfelbaum" <mapfelba@redhat.com>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	"Richard Henderson" <richard.henderson@linaro.org>,
	qemu-devel@nongnu.org, "Peter Xu" <peterx@redhat.com>,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	"Greg Kurz" <groug@kaod.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Murilo Opsfelder Araujo" <muriloo@linux.ibm.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Igor Mammedov" <imammedo@redhat.com>
Subject: Re: [PATCH v6 10/15] hostmem: Wire up RAM_NORESERVE via "reserve" property
Date: Fri, 23 Apr 2021 16:53:18 +0200	[thread overview]
Message-ID: <87sg3hdpcx.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <884e43d0-36c1-f897-3a4e-03d96a9861c0@redhat.com> (David Hildenbrand's message of "Fri, 23 Apr 2021 16:21:22 +0200")

David Hildenbrand <david@redhat.com> writes:

> On 23.04.21 14:10, Markus Armbruster wrote:
>> David Hildenbrand <david@redhat.com> writes:
>> 
>>>>> 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;
>>>>> +    }
>>>>
>>>> Aha.  Shouldn't this be documented in qom.json?
>>>
>>> Whoops, skipped that comment. Can add it if that's the place to
>>> document that.
>> 
>> Yes, please.  .json doc comments is where we document the external
>> interface.
>> 
>
> What about something like this:
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index e9b86893a5..4fa3137aab 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -559,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',

Thanks!

Reviewed-by: Markus Armbruster <armbru@redhat.com>