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

[Eduardo Habkost]

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.

-- 
Eduardo