Re: [PATCH v4 10/11] 9p: darwin: meson: Allow VirtFS on Darwin

[Will Cohen <wwcohen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3200 bytes --]

On Mon, Feb 7, 2022 at 9:27 AM Christian Schoenebeck <qemu_oss@crudebyte.com>
wrote:

> On Sonntag, 6. Februar 2022 21:07:18 CET Will Cohen wrote:
> > From: Keno Fischer <keno@juliacomputing.com>
> >
> > Signed-off-by: Keno Fischer <keno@juliacomputing.com>
> > [Michael Roitzsch: - Rebase for NixOS]
> > Signed-off-by: Michael Roitzsch <reactorcontrol@icloud.com>
> > [Will Cohen: - Rebase to master]
> > Signed-off-by: Will Cohen <wwcohen@gmail.com>
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > [Will Cohen: - Add check for pthread_fchdir_np to virtfs]
> > Signed-off-by: Will Cohen <wwcohen@gmail.com>
> > ---
> >  fsdev/meson.build |  1 +
> >  meson.build       | 14 ++++++++++----
> >  2 files changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/fsdev/meson.build b/fsdev/meson.build
> > index adf57cc43e..b632b66348 100644
> > --- a/fsdev/meson.build
> > +++ b/fsdev/meson.build
> > @@ -7,6 +7,7 @@ fsdev_ss.add(when: ['CONFIG_FSDEV_9P'], if_true: files(
> >    'qemu-fsdev.c',
> >  ), if_false: files('qemu-fsdev-dummy.c'))
> >  softmmu_ss.add_all(when: 'CONFIG_LINUX', if_true: fsdev_ss)
> > +softmmu_ss.add_all(when: 'CONFIG_DARWIN', if_true: fsdev_ss)
> >
> >  if have_virtfs_proxy_helper
> >    executable('virtfs-proxy-helper',
> > diff --git a/meson.build b/meson.build
> > index 5f43355071..6b4adf7e15 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1421,17 +1421,23 @@ if not get_option('dbus_display').disabled()
> >    endif
> >  endif
> >
> > -have_virtfs = (targetos == 'linux' and
> > +if targetos == 'darwin' and cc.has_function('pthread_fchdir_np')
> > +  have_virtfs = have_system
>
> As you are going for a v5 anyway: I would add an error message here if
> pthread_fchdir_np() is not available. Because it is a bit frustrating for
> users if their options silently got ignored without any indication why.
>
> > +else
> > +  have_virtfs = (targetos == 'linux' and
> >      have_system and
> >      libattr.found() and
> >      libcap_ng.found())
> > +endif
> >
> > -have_virtfs_proxy_helper = have_virtfs and have_tools
> > +have_virtfs_proxy_helper = targetos == 'linux' and have_virtfs and
> > have_tools
> >
> >  if get_option('virtfs').enabled()
> >    if not have_virtfs
> > -    if targetos != 'linux'
> > -      error('virtio-9p (virtfs) requires Linux')
> > +    if targetos != 'linux' and targetos != 'darwin'
> > +      error('virtio-9p (virtfs) requires Linux or Darwin')
> > +    elif targetos == 'darwin' and not
> cc.has_function('pthread_fchdir_np')
> > +      error('virtio-9p (virtfs) on Darwin requires the presence of
> > pthread_fchdir_np')


Does the error message here suffice for that need? Right now if they're
running a system without pthread_fchdir_np and don't specify the option, I
think it'll just quietly disable, but if they --enable-virtfs and the
function isn't there, they should get a note. I assume this is better, so
that the ability to compile isn't contingent on having the latest OS, even
if full support for older OSes isn't provided.


> elif not libcap_ng.found() or not libattr.found()
> >        error('virtio-9p (virtfs) requires libcap-ng-devel and
> > libattr-devel') elif not have_system
>
>
>

[-- Attachment #2: Type: text/html, Size: 4710 bytes --]