Re: [PATCH v4 09/14] util/mmap-alloc: Pass flags instead of separate bools to qemu_ram_mmap()

[David Hildenbrand <david@redhat.com>]

On 23.03.21 21:49, Peter Xu wrote:
> On Fri, Mar 19, 2021 at 11:12:25AM +0100, David Hildenbrand wrote:
>> Let's pass flags instead of bools to prepare for passing other flags and
>> update the documentation of qemu_ram_mmap(). Introduce new QEMU_MAP_
>> flags that abstract the mmap() PROT_ and MAP_ flag handling and simplify
>> it.
>>
>> We expose only flags that are currently supported by qemu_ram_mmap().
>> Maybe, we'll see qemu_mmap() in the future as well that can implement these
>> flags.
>>
>> Note: We don't use MAP_ flags as some flags (e.g., MAP_SYNC) are only
>> defined for some systems and we want to always be able to identify
>> these flags reliably inside qemu_ram_mmap() -- for example, to properly
>> warn when some future flags are not available or effective on a system.
>> Also, this way we can simplify PROT_ handling as well.
>>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>   include/qemu/mmap-alloc.h | 16 +++++++++-------
>>   include/qemu/osdep.h      | 18 ++++++++++++++++++
>>   softmmu/physmem.c         |  8 +++++---
>>   util/mmap-alloc.c         | 15 ++++++++-------
>>   util/oslib-posix.c        |  3 ++-
>>   5 files changed, 42 insertions(+), 18 deletions(-)
>>
>> diff --git a/include/qemu/mmap-alloc.h b/include/qemu/mmap-alloc.h
>> index 456ff87df1..a60a2085b3 100644
>> --- a/include/qemu/mmap-alloc.h
>> +++ b/include/qemu/mmap-alloc.h
>> @@ -7,18 +7,22 @@ size_t qemu_fd_getpagesize(int fd);
>>   size_t qemu_mempath_getpagesize(const char *mem_path);
>>   
>>   /**
>> - * qemu_ram_mmap: mmap the specified file or device.
>> + * qemu_ram_mmap: mmap anonymous memory, the specified file or device.
>> + *
>> + * mmap() abstraction to map guest RAM, simplifying flag handling, taking
>> + * care of alignment requirements and installing guard pages.
>>    *
>>    * Parameters:
>>    *  @fd: the file or the device to mmap
>>    *  @size: the number of bytes to be mmaped
>>    *  @align: if not zero, specify the alignment of the starting mapping address;
>>    *          otherwise, the alignment in use will be determined by QEMU.
>> - *  @readonly: true for a read-only mapping, false for read/write.
>> - *  @shared: map has RAM_SHARED flag.
>> - *  @is_pmem: map has RAM_PMEM flag.
>> + *  @qemu_map_flags: QEMU_MAP_* flags
>>    *  @map_offset: map starts at offset of map_offset from the start of fd
>>    *
>> + * Internally, MAP PRIVATE, MAP_ANONYMOUS and MAP_SHARED_VALIDATE are set
>                       ^
>                       |
>                       +---- underscore
> 

Nice catch, thanks :)


-- 
Thanks,

David / dhildenb