On 2/24/21 7:52 AM, Kevin Wolf wrote:
> This adds a QAPI schema for the properties of the memory-backend-*
> objects.
>
> HostMemPolicy has to be moved to an include file that can be used by the
> storage daemon, too, because ObjectOptions must be the same in all
> binaries if we don't want to compile the whole code multiple times.
>
> Signed-off-by: Kevin Wolf <kwolf@xxxxxxxxxx>
> ---
> qapi/common.json | 20 ++++++++
> qapi/machine.json | 22 +--------
> qapi/qom.json | 118 +++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 138 insertions(+), 22 deletions(-)
>
> +++ b/qapi/qom.json
> +##
> +# @MemoryBackendProperties:
> +#
> +# Properties for objects of classes derived from memory-backend.
> +#
> +# @merge: if true, mark the memory as mergeable (default depends on the machine
> +# type)
> +#
> +# @dump: if true, include the memory in core dumps (default depends on the
> +# machine type)
Interesting choice to flip the description text from its previous
wording, but fine by me:
object_class_property_set_description(oc, "dump",
"Set to 'off' to exclude from core dump");
> +#
> +# @host-nodes: the list of NUMA host nodes to bind the memory to
> +#
> +# @policy: the NUMA policy (default: 'default')
> +#
> +# @prealloc: if true, preallocate memory (default: false)
Not quite in the same order as
backends/hostmem.c:host_memory_backend_class_init() (alphabetic here
instead of matching the C code declaration order), but that doesn't
impact QMP semantics, and I was able to match everything up in the end.
> +#
> +# @prealloc-threads: number of CPU threads to use for prealloc (default: 1)
> +#
> +# @share: if false, the memory is private to QEMU; if true, it is shared
> +# (default: false)
> +#
> +# @size: size of the memory region in bytes
> +#
> +# @x-use-canonical-path-for-ramblock-id: if true, the canoncial path is used
> +# for ramblock-id. Disable this for 4.0
> +# machine types or older to allow
> +# migration with newer QEMU versions.
> +# (default: false generally, but true
> +# for machine types <= 4.0)
The comment in the C code mentions that in spite of the x- prefix, we
have to treat this as a stable interface until 4.0 machines disappear.
Do we need any of that sentiment in the documentation here?
> +#
> +# Since: 2.1
> +##
> +{ 'struct': 'MemoryBackendProperties',
> + 'data': { '*dump': 'bool',
> + '*host-nodes': ['uint16'],
> + '*merge': 'bool',
> + '*policy': 'HostMemPolicy',
> + '*prealloc': 'bool',
> + '*prealloc-threads': 'uint32',
> + '*share': 'bool',
> + 'size': 'size',
> + '*x-use-canonical-path-for-ramblock-id': 'bool' } }
> +
> +##
> +# @MemoryBackendFileProperties:
> +#
> +# Properties for memory-backend-file objects.
> +#
> +# @align: the base address alignment when QEMU mmap(2) @mem-path. Some
> +# backend store specified by @mem-path requires an alignment different
Grammar feels off. Would it read better as
...when QEMU mmap(2)s @mem-path. Some backend stores specified by
@mem-path require an...
> +# than the default one used by QEMU, e.g. the device DAX /dev/dax0.0
> +# requires 2M alignment rather than 4K. In such cases, users can
> +# specify the required alignment via this option.
> +# 0 selects a default alignment (currently the page size). (default: 0)
Again, not in the same order as
backends/hostmem-file.c:file_backend_class_init(), but it matches up.
> +#
> +# @discard-data: if true, the file contents can be destroyed when QEMU exits,
> +# to avoid unnecessarily flushing data to the backing file. Note
> +# that ``discard-data`` is only an optimization, and QEMU might
> +# not discard file contents if it aborts unexpectedly or is
> +# terminated using SIGKILL. (default: false)
> +#
> +# @mem-path: the path to either a shared memory or huge page filesystem mount
> +#
> +# @pmem: specifies whether the backing file specified by @mem-path is in
> +# host persistent memory that can be accessed using the SNIA NVM
> +# programming model (e.g. Intel NVDIMM).
> +#
> +# @readonly: if true, the backing file is opened read-only; if false, it is
> +# opened read-write. (default: false)
> +#
> +# Since: 2.1
> +##
> +{ 'struct': 'MemoryBackendFileProperties',
> + 'base': 'MemoryBackendProperties',
> + 'data': { '*align': 'size',
> + '*discard-data': 'bool',
> + 'mem-path': 'str',
> + '*pmem': 'bool',
To match the C code, this should be
'*pmem': { 'type':'bool', 'if':'defined(CONFIG_LIBPMEM)' },
> + '*readonly': 'bool' } }
> +
> +##
> +# @MemoryBackendMemfdProperties:
> +#
> +# Properties for memory-backend-memfd objects.
> +#
> +# The @share boolean option is true by default with memfd.
> +#
> +# @hugetlb: if true, the file to be created resides in the hugetlbfs filesystem
> +# (default: false)
> +#
> +# @hugetlbsize: the hugetlb page size on systems that support multiple hugetlb
> +# page sizes (it must be a power of 2 value supported by the
> +# system). 0 selects a default page size. This option is ignored
> +# if @hugetlb is false. (default: 0)
> +#
> +# @seal: if true, create a sealed-file, which will block further resizing of
> +# the memory (default: true)
> +#
> +# Since: 2.12
> +##
> +{ 'struct': 'MemoryBackendMemfdProperties',
> + 'base': 'MemoryBackendProperties',
> + 'data': { '*hugetlb': 'bool',
> + '*hugetlbsize': 'size',
> + '*seal': 'bool' } }
backends/hostmem-memfd.c makes 'hugetlb' and 'hugetlbsize' conditional
on qemu_memfd_check(MFD_HUGETLB), and only registers the overal type
based on qemu_memfd_check(MFD_ALLOW_SEALING). In turn, qemu_memfd_check
returns false except for CONFIG_LINUX,...
> +
> ##
> # @ObjectType:
> #
> @@ -287,7 +395,10 @@
> 'cryptodev-backend-builtin',
> 'cryptodev-vhost-user',
> 'dbus-vmstate',
> - 'iothread'
> + 'iothread',
> + 'memory-backend-file',
> + 'memory-backend-memfd',
> + 'memory-backend-ram'
> ] }
>
> ##
> @@ -314,7 +425,10 @@
> 'cryptodev-backend-builtin': 'CryptodevBackendProperties',
> 'cryptodev-vhost-user': 'CryptodevVhostUserProperties',
> 'dbus-vmstate': 'DBusVMStateProperties',
> - 'iothread': 'IothreadProperties'
> + 'iothread': 'IothreadProperties',
> + 'memory-backend-file': 'MemoryBackendFileProperties',
> + 'memory-backend-memfd': 'MemoryBackendMemfdProperties',
...so I'm wondering if this branch should be:
'memory-backend-memfd', { 'type':'MemoryBackendMemfdProperties',
'if': 'defined(CONFIG_LINUX)' },
and whether we are risking problems by always having the 'hugetlb*'
fields even when the runtime does not register them.
> + 'memory-backend-ram': 'MemoryBackendProperties'
> } }
>
> ##
>
Because of my questions on conditional compilation, I'm not comfortable
with R-b yet.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
