Re: [Qemu-devel] [PATCH v2 1/4] hostmem-memfd: disable for systems wihtout sealing support

[Ilya Maximets]

So, can we have any conclusion about this patch and the series?

Best regards, Ilya Maximets.

On 05.01.2019 5:43, Eduardo Habkost wrote:
> On Tue, Dec 11, 2018 at 04:48:23PM +0100, Igor Mammedov wrote:
>> On Tue, 11 Dec 2018 13:29:19 +0300
>> Ilya Maximets <address@hidden> wrote:
>>
>> CCing libvirt folk for an opinion
>>
>>> On 10.12.2018 19:18, Igor Mammedov wrote:
>>>> On Tue, 27 Nov 2018 16:50:27 +0300
>>>> Ilya Maximets <address@hidden> wrote:
>>>>
>>>> s/wihtout/without/ in subj
>>>>   
>>>>> If seals are not supported, memfd_create() will fail.
>>>>> Furthermore, there is no way to disable it in this case because
>>>>> '.seal' property is not registered.
>>>>>
>>>>> This issue leads to vhost-user-test failures on RHEL 7.2:
>>>>>
>>>>>   qemu-system-x86_64: -object memory-backend-memfd,id=mem,size=2M,: \
>>>>>                       failed to create memfd: Invalid argument
>>>>>
>>>>> and actually breaks the feature on such systems.
>>>>>
>>>>> Let's restrict memfd backend to systems with sealing support.
>>>>>
>> [...]
>>>>> @@ -177,7 +175,7 @@ static const TypeInfo memfd_backend_info = {
>>>>>  
>>>>>  static void register_types(void)
>>>>>  {
>>>>> -    if (qemu_memfd_check(0)) {
>>>>> +    if (qemu_memfd_check(MFD_ALLOW_SEALING)) {
>>>>>          type_register_static(&memfd_backend_info);  
>>>> that would either lead to not clear error that type doesn't exist.
>>>> it could be better to report sensible error from 
>>>> memfd_backend_memory_alloc() if
>>>> the feature is required but not supported by host   
>>>
>>> I'm not sure, but this could break the libvirt capability discovering.
>>>
>>> Current patch changes behaviour probably only for RHEL/CentOS 7.2.
>>> All other systems are not affected. Do you think that we need to
>>> change behaviour on all the systems?
>> you are changing behavior anyways, so when users start getting
>> on some of 'All other systems' start getting 'type doesn't exist'
>> error, they won't have a clue what's wrong. In case where we are
>> fixing broken defaults, shouldn't we at least do it the way that
>> would inform user about misconfiguration.
>>
>> But I'm not insisting since memfd is fairly new, it might be fine
>> for device to just disappear.
> 
> (Sorry for taking so long to reply on this series.  I couldn't
> review the code yet.)
> 
> I'd like to make the QOM type hierarchy static, and not depend on
> any runtime host capability checks.  But this is not a new
> problem in the code, so I don't think it should block this
> series.
>