[PATCH] tools/meson.build: Error on enabling virtiofsd and have_system is false

Mahmoud Mandour posted 1 patch 3 years ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20210428133507.52066-1-ma.mandourr@gmail.com
tools/meson.build | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
[PATCH] tools/meson.build: Error on enabling virtiofsd and have_system is false
Posted by Mahmoud Mandour 3 years ago
Previously, on configuring with --enable-virtiofsd and specifying
a target list that does not contain a full-system emulation target,
a spurious error message is emitted. This patch introduces a
meaningful error message for such case.

Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
---
 tools/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/meson.build b/tools/meson.build
index 3e5a0abfa2..f6a4ced2f4 100644
--- a/tools/meson.build
+++ b/tools/meson.build
@@ -5,7 +5,9 @@ have_virtiofsd = (targetos == 'linux' and
     'CONFIG_VHOST_USER' in config_host)
 
 if get_option('virtiofsd').enabled()
-  if not have_virtiofsd
+  if not have_system
+    error('virtiofsd requires full-system emulation target(s)')
+  elif not have_virtiofsd
     if targetos != 'linux'
       error('virtiofsd requires Linux')
     elif not seccomp.found() or not libcap_ng.found()
-- 
2.25.1


Re: [PATCH] tools/meson.build: Error on enabling virtiofsd and have_system is false
Posted by Connor Kuehl 3 years ago
On 4/28/21 8:35 AM, Mahmoud Mandour wrote:
> Previously, on configuring with --enable-virtiofsd and specifying
> a target list that does not contain a full-system emulation target,
> a spurious error message is emitted. This patch introduces a
> meaningful error message for such case.
> 
> Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> ---
>  tools/meson.build | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/meson.build b/tools/meson.build
> index 3e5a0abfa2..f6a4ced2f4 100644
> --- a/tools/meson.build
> +++ b/tools/meson.build
> @@ -5,7 +5,9 @@ have_virtiofsd = (targetos == 'linux' and
>      'CONFIG_VHOST_USER' in config_host)
>  
>  if get_option('virtiofsd').enabled()
> -  if not have_virtiofsd
> +  if not have_system
> +    error('virtiofsd requires full-system emulation target(s)')

I am not entirely sure if this is true. The error message before this
patch is applied is:

	../tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd
	requires libcap-ng-devel and seccomp-devel

From what I know about virtiofsd, I know it definitely depends on those
two things.

Is it possible that the error here is that the top-level meson.build is
not properly collecting the seccomp and libcap-ng dependencies if the
configure invocation doesn't require a system emulation target?

Thanks,

Connor

> +  elif not have_virtiofsd
>      if targetos != 'linux'
>        error('virtiofsd requires Linux')
>      elif not seccomp.found() or not libcap_ng.found()
> 


Re: [PATCH] tools/meson.build: Error on enabling virtiofsd and have_system is false
Posted by Mahmoud Mandour 3 years ago
On Wed, Apr 28, 2021 at 3:56 PM Connor Kuehl <ckuehl@redhat.com> wrote:

> On 4/28/21 8:35 AM, Mahmoud Mandour wrote:
> > Previously, on configuring with --enable-virtiofsd and specifying
> > a target list that does not contain a full-system emulation target,
> > a spurious error message is emitted. This patch introduces a
> > meaningful error message for such case.
> >
> > Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> > ---
> >  tools/meson.build | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/meson.build b/tools/meson.build
> > index 3e5a0abfa2..f6a4ced2f4 100644
> > --- a/tools/meson.build
> > +++ b/tools/meson.build
> > @@ -5,7 +5,9 @@ have_virtiofsd = (targetos == 'linux' and
> >      'CONFIG_VHOST_USER' in config_host)
> >
> >  if get_option('virtiofsd').enabled()
> > -  if not have_virtiofsd
> > +  if not have_system
> > +    error('virtiofsd requires full-system emulation target(s)')
>
> I am not entirely sure if this is true. The error message before this
> patch is applied is:
>
>         ../tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd
>         requires libcap-ng-devel and seccomp-devel
>
> From what I know about virtiofsd, I know it definitely depends on those
> two things.
>
> Is it possible that the error here is that the top-level meson.build is
> not properly collecting the seccomp and libcap-ng dependencies if the
> configure invocation doesn't require a system emulation target?
>
> Thanks,
>
> Connor
>
> > +  elif not have_virtiofsd
> >      if targetos != 'linux'
> >        error('virtiofsd requires Linux')
> >      elif not seccomp.found() or not libcap_ng.found()
> >
>
>
I also thought that this is the case since I also specifically get this
error message
if I enable virtiofsd and specify a target list with only Linux-user
targets while nothing
in tools/meson.build specifies so. But I think that even if it correctly
managed the
dependencies it would include and build virtiofsd unnecessarily and that's
not what we want(?)

Thanks,
Mahmoud
Re: [PATCH] tools/meson.build: Error on enabling virtiofsd and have_system is false
Posted by Connor Kuehl 3 years ago
On 4/28/21 9:13 AM, Mahmoud Mandour wrote:
>> I am not entirely sure if this is true. The error message before this
>> patch is applied is:
>>
>>         ../tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd
>>         requires libcap-ng-devel and seccomp-devel
>>
>> From what I know about virtiofsd, I know it definitely depends on those
>> two things.
>>
>> Is it possible that the error here is that the top-level meson.build is
>> not properly collecting the seccomp and libcap-ng dependencies if the
>> configure invocation doesn't require a system emulation target?
> I also thought that this is the case since I also specifically get this
> error message
> if I enable virtiofsd and specify a target list with only Linux-user
> targets while nothing
> in tools/meson.build specifies so. But I think that even if it correctly
> managed the
> dependencies it would include and build virtiofsd unnecessarily and that's
> not what we want(?)

I think that's exactly what we want for the default case because
virtiofsd is enabled by default (../configure --help).

Even if the virtiofsd dependencies are satisfied, if one doesn't want to
build virtiofsd, they can pass --disable-virtiofsd to the configure
invocation.

Connor


Re: [PATCH] tools/meson.build: Error on enabling virtiofsd and have_system is false
Posted by Mahmoud Mandour 3 years ago
On Wed, Apr 28, 2021 at 4:29 PM Connor Kuehl <ckuehl@redhat.com> wrote:

> On 4/28/21 9:13 AM, Mahmoud Mandour wrote:
> >> I am not entirely sure if this is true. The error message before this
> >> patch is applied is:
> >>
> >>         ../tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd
> >>         requires libcap-ng-devel and seccomp-devel
> >>
> >> From what I know about virtiofsd, I know it definitely depends on those
> >> two things.
> >>
> >> Is it possible that the error here is that the top-level meson.build is
> >> not properly collecting the seccomp and libcap-ng dependencies if the
> >> configure invocation doesn't require a system emulation target?
> > I also thought that this is the case since I also specifically get this
> > error message
> > if I enable virtiofsd and specify a target list with only Linux-user
> > targets while nothing
> > in tools/meson.build specifies so. But I think that even if it correctly
> > managed the
> > dependencies it would include and build virtiofsd unnecessarily and
> that's
> > not what we want(?)
>
> I think that's exactly what we want for the default case because
> virtiofsd is enabled by default (../configure --help).

Even if the virtiofsd dependencies are satisfied, if one doesn't want to
> build virtiofsd, they can pass --disable-virtiofsd to the configure
> invocation.
>
> Connor
>
>
That makes sense, I now understand. Thank you.

Mahmoud
Re: [PATCH] tools/meson.build: Error on enabling virtiofsd and have_system is false
Posted by Philippe Mathieu-Daudé 3 years ago
On 4/28/21 3:56 PM, Connor Kuehl wrote:
> On 4/28/21 8:35 AM, Mahmoud Mandour wrote:
>> Previously, on configuring with --enable-virtiofsd and specifying
>> a target list that does not contain a full-system emulation target,
>> a spurious error message is emitted. This patch introduces a
>> meaningful error message for such case.
>>
>> Signed-off-by: Mahmoud Mandour <ma.mandourr@gmail.com>
>> ---
>>  tools/meson.build | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/meson.build b/tools/meson.build
>> index 3e5a0abfa2..f6a4ced2f4 100644
>> --- a/tools/meson.build
>> +++ b/tools/meson.build
>> @@ -5,7 +5,9 @@ have_virtiofsd = (targetos == 'linux' and
>>      'CONFIG_VHOST_USER' in config_host)
>>  
>>  if get_option('virtiofsd').enabled()
>> -  if not have_virtiofsd
>> +  if not have_system
>> +    error('virtiofsd requires full-system emulation target(s)')
> 
> I am not entirely sure if this is true. The error message before this
> patch is applied is:
> 
> 	../tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd
> 	requires libcap-ng-devel and seccomp-devel
> 
> From what I know about virtiofsd, I know it definitely depends on those
> two things.
> 
> Is it possible that the error here is that the top-level meson.build is
> not properly collecting the seccomp and libcap-ng dependencies if the
> configure invocation doesn't require a system emulation target?

This series should fix your problems:
https://patchew.org/QEMU/20210428144813.417170-1-philmd@redhat.com/