Re: [PATCH for-9.1 v2 09/11] hostmem: add a new memory backend based on POSIX shm_open()

[David Hildenbrand <david@redhat.com>]

>>>
>>> So I thought that for now we only support the "anonymous" mode, and if
>>> in the future we have a use case where the user wants to specify the
>>> name, we can add it later.
>>
>> Okay, so for now you really only want an anonymous fd that behaves like
>> a memfd, got it.
>>
>> Likely we should somehow fail if the "POSIX shared memory object"
>> already exists. Hmm.
> 
> `O_CREAT | O_EXCL` should provide just this behavior.
>   From shm_open(3) manpage:
> 
>       O_EXCL  If O_CREAT was also specified, and a shared memory object
>               with the given name already exists, return an error.  The
>               check for the existence of the object, and its creation if
>               it does not exist, are performed atomically.
> 

Cool!

>>
>>>
>>> That said, if you think it's already useful from the beginning, I can
>>> add the name as an optional parameter.
>>>
>>>>
>>>> I'm also not quite sure if "host_memory_backend_get_name()" should be
>>>> used for the purpose here.
>>>
>>> What problem do you see? As an alternative I thought of a static
>>> counter.
>>
>> If it's really just an "internal UUID", as we'll remove the file using
>> shm_unlink(), then the name in fact shouldn't matter and this would be
>> fine. Correct?
> 
> Right. It's a name that will only "appear" in the system for a very
> short time since we call shm_unlink() right after shm_open(). So I just
> need the unique name to prevent several QEMUs launched at the same time
> from colliding with the name.

Okay, essentially emulating tmpfile(), but for weird shmem_open() :)

> 
>>
>> So I assume if we ever want to have non-anonymous fds here, we'd pass
>> in a new property "name", and change the logic how we open/unlink.
>> Makes sense.
> 
> Exactly! I can clarify this in the commit description as well.
> 
> Thanks for the helpful comments!
> If there is anything I need to change for v3, please tell me ;-)

Besides some patch description extension, makes sense to me :)

-- 
Cheers,

David / dhildenb