[PATCH] meson: don't build vstorage where mntent.h is not present

Nikolay Shirokovskiy posted 1 patch 3 years, 3 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1611063695-280240-1-git-send-email-nshirokovskiy@virtuozzo.com
meson.build | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
[PATCH] meson: don't build vstorage where mntent.h is not present
Posted by Nikolay Shirokovskiy 3 years, 3 months ago
This should fix CI error:

    ../dist-unpack/libvirt-7.1.0/src/storage/storage_backend_vstorage.c:10:10: fatal error: 'mntent.h' file not found
    #include <mntent.h>
    ^~~~~~~~~~

on freebsd and mac.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
---
 meson.build | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index e3e7ff7..a6b6169 100644
--- a/meson.build
+++ b/meson.build
@@ -1957,8 +1957,20 @@ if conf.has('WITH_LIBVIRTD')
   endif
 
   if not get_option('storage_vstorage').disabled()
-    use_storage = true
-    conf.set('WITH_STORAGE_VSTORAGE', 1)
+    vstorage_enable = true
+
+    if not cc.has_header('mntent.h')
+      if get_option('storage_fs').enabled()
+        error('<mntent.h> is required for the FS storage driver')
+      else
+        vstorage_enable = false
+      endif
+    endif
+
+    if vstorage_enable
+      use_storage = true
+      conf.set('WITH_STORAGE_VSTORAGE', 1)
+    endif
   endif
 
   if not get_option('storage_zfs').disabled()
-- 
1.8.3.1

Re: [PATCH] meson: don't build vstorage where mntent.h is not present
Posted by Pavel Hrdina 3 years, 3 months ago
On Tue, Jan 19, 2021 at 04:41:35PM +0300, Nikolay Shirokovskiy wrote:
> This should fix CI error:
> 
>     ../dist-unpack/libvirt-7.1.0/src/storage/storage_backend_vstorage.c:10:10: fatal error: 'mntent.h' file not found
>     #include <mntent.h>
>     ^~~~~~~~~~
> 
> on freebsd and mac.
> 
> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> ---
>  meson.build | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index e3e7ff7..a6b6169 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1957,8 +1957,20 @@ if conf.has('WITH_LIBVIRTD')
>    endif
>  
>    if not get_option('storage_vstorage').disabled()
> -    use_storage = true
> -    conf.set('WITH_STORAGE_VSTORAGE', 1)
> +    vstorage_enable = true
> +
> +    if not cc.has_header('mntent.h')

This makes me question if it makes sense to build vstorage for anything
else then linux? It looks like that on FreeBSD or macOS it will be never
enabled and we already disable libvirtd and all storage drivers for
windows so we might as well make this condition

    if host_machine.system() != 'linux'

and claim that vstorage is supported only on linux.

I see that the check is inspired by FS storage driver but if mntent.h is
not available or difficult to get on FreeBSD or macOS we could make it
easier for users instead of having them trying to get mntent.h.

> +      if get_option('storage_fs').enabled()
> +        error('<mntent.h> is required for the FS storage driver')

This should probably say "Virtuozzo storage driver".

Pavel

> +      else
> +        vstorage_enable = false
> +      endif
> +    endif
> +
> +    if vstorage_enable
> +      use_storage = true
> +      conf.set('WITH_STORAGE_VSTORAGE', 1)
> +    endif
>    endif
>  
>    if not get_option('storage_zfs').disabled()
> -- 
> 1.8.3.1
> 
Re: [PATCH] meson: don't build vstorage where mntent.h is not present
Posted by Nikolay Shirokovskiy 3 years, 3 months ago
On Tue, Jan 19, 2021 at 5:59 PM Pavel Hrdina <phrdina@redhat.com> wrote:

> On Tue, Jan 19, 2021 at 04:41:35PM +0300, Nikolay Shirokovskiy wrote:
> > This should fix CI error:
> >
> >
>  ../dist-unpack/libvirt-7.1.0/src/storage/storage_backend_vstorage.c:10:10:
> fatal error: 'mntent.h' file not found
> >     #include <mntent.h>
> >     ^~~~~~~~~~
> >
> > on freebsd and mac.
> >
> > Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
> > ---
> >  meson.build | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/meson.build b/meson.build
> > index e3e7ff7..a6b6169 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -1957,8 +1957,20 @@ if conf.has('WITH_LIBVIRTD')
> >    endif
> >
> >    if not get_option('storage_vstorage').disabled()
> > -    use_storage = true
> > -    conf.set('WITH_STORAGE_VSTORAGE', 1)
> > +    vstorage_enable = true
> > +
> > +    if not cc.has_header('mntent.h')
>
> This makes me question if it makes sense to build vstorage for anything
> else then linux? It looks like that on FreeBSD or macOS it will be never
> enabled and we already disable libvirtd and all storage drivers for
> windows so we might as well make this condition
>
>     if host_machine.system() != 'linux'
>
> and claim that vstorage is supported only on linux.
>
> I see that the check is inspired by FS storage driver but if mntent.h is
> not available or difficult to get on FreeBSD or macOS we could make it
> easier for users instead of having them trying to get mntent.h.
>

Ok.


>
> > +      if get_option('storage_fs').enabled()
> > +        error('<mntent.h> is required for the FS storage driver')
>
> This should probably say "Virtuozzo storage driver".
>
>
Yep, fixing CI is a bit of a hurry :)

Nikolay



>
> > +      else
> > +        vstorage_enable = false
> > +      endif
> > +    endif
> > +
> > +    if vstorage_enable
> > +      use_storage = true
> > +      conf.set('WITH_STORAGE_VSTORAGE', 1)
> > +    endif
> >    endif
> >
> >    if not get_option('storage_zfs').disabled()
> > --
> > 1.8.3.1
> >
>