From: Markus Armbruster <armbru@redhat.com>
To: Stefano Garzarella <sgarzare@redhat.com>
Cc: qemu-devel@nongnu.org, "Eric Blake" <eblake@redhat.com>,
"Daniel P. Berrangé" <berrange@redhat.com>,
"Eduardo Habkost" <eduardo@habkost.net>,
"Paolo Bonzini" <pbonzini@redhat.com>
Subject: Re: [PATCH] qapi: clarify that the default is backend dependent
Date: Mon, 03 Jun 2024 11:34:10 +0200 [thread overview]
Message-ID: <877cf61ib1.fsf@pond.sub.org> (raw)
In-Reply-To: <20240523133302.103858-1-sgarzare@redhat.com> (Stefano Garzarella's message of "Thu, 23 May 2024 15:33:02 +0200")
Stefano Garzarella <sgarzare@redhat.com> writes:
> The default value of the @share option of the @MemoryBackendProperties
> eally depends on the backend type, so let's document it explicitly and
> add the default value where it was missing.
>
> Cc: David Hildenbrand <david@redhat.com>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
> I followed how we document @share in memfd and epc, but I don't like it
> very much, I just can't think of a better way, so if you have a suggestion
> I can change them in all of them.
>
> Thanks,
> Stefano
> ---
> qapi/qom.json | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 38dde6d785..8463bd32a2 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -600,7 +600,7 @@
##
# @MemoryBackendProperties:
#
# Properties for objects of classes derived from memory-backend.
#
[...]
> # preallocation threads (default: none) (since 7.2)
> #
> # @share: if false, the memory is private to QEMU; if true, it is
> -# shared (default: false)
> +# shared (default depends on the backend type)
Note for later: the backends are the branches of ObjectOptions that use
MemoryBackendProperties as branch type or as base of their branch type.
These are
memory-backend-epc (uses MemoryBackendEpcProperties)
memory-backend-file (uses MemoryBackendFileProperties)
memory-backend-memfd (uses MemoryBackendMemfdProperties)
memory-backend-ram (uses MemoryBackendProperties)
> #
> # @reserve: if true, reserve swap space (or huge pages) if applicable
> # (default: true) (since 6.1)
> @@ -639,6 +639,8 @@
> #
> # Properties for memory-backend-file objects.
> #
> +# The @share boolean option is false by default with file.
> +#
> # @align: the base address alignment when QEMU mmap(2)s @mem-path.
> # Some backend stores specified by @mem-path require an alignment
> # different than the default one used by QEMU, e.g. the device DAX
As stated in the commit message, this matches existing documentation in
memory-backend-epc
# The @share boolean option is true by default with epc
and memory-backend-memfd
# The @share boolean option is true by default with memfd.
I think "with FOO" could be clearer. Perhaps something like "with
backend 'memory-backend-FOO'.
However, even with your patch, we're still missing memory-backend-ram.
I can see two solutions:
1. Create MemoryBackendRamProperties just to have a place for
documenting @share's default.
2. Document @share's default right where it's defined, roughly like
this:
# @share: if false, the memory is private to QEMU; if true, it is
# shared (default false for backends memory-backend-file and
# memory-backend-ram, true for backends memory-backend-epc and
# memory-backend-memfd)
CON: we need to remember to update this whenever we add another backend.
PRO: generated documentation is better, in my opinion.
Thoughts?
next prev parent reply other threads:[~2024-06-03 9:34 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-23 13:33 [PATCH] qapi: clarify that the default is backend dependent Stefano Garzarella
2024-06-03 9:34 ` Markus Armbruster [this message]
2024-06-04 13:21 ` Stefano Garzarella
2024-06-04 14:58 ` Markus Armbruster
2024-06-06 14:06 ` Stefano Garzarella
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=877cf61ib1.fsf@pond.sub.org \
--to=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=eblake@redhat.com \
--cc=eduardo@habkost.net \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=sgarzare@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.