[PATCH v2 0/2] virtiofsd: Meson build fix

[PATCH v2 0/2] virtiofsd: Meson build fix

[Philippe Mathieu-Daudé]

Meson fix to allow building virtiofsd without sysemu/tools.

Since v1:
- reworked meson (Richard)
- added CI job (Dave)

Regards,

Phil.

Supersedes: <20210428144813.417170-1-philmd@redhat.com>

Philippe Mathieu-Daudé (2):
  meson: Select 'have_system' when virtiofsd is enabled
  gitlab-ci: Add a job to build virtiofsd standalone

 meson.build    |  2 ++
 .gitlab-ci.yml | 13 +++++++++++++
 2 files changed, 15 insertions(+)

-- 
2.26.3

[PATCH v2 1/2] meson: Select 'have_system' when virtiofsd is enabled

[Philippe Mathieu-Daudé]

When not explicitly select a sysemu target and building virtiofsd,
the seccomp/cap-ng libraries are not resolved, leading to this error:

  $ configure --target-list=i386-linux-user --disable-tools --enable-virtiofsd
  tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd requires libcap-ng-devel and seccomp-devel

Fix by enabling sysemu (have_system) when virtiofsd is built.

Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 meson.build | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/meson.build b/meson.build
index c6f4b0cf5e8..f858935ad95 100644
--- a/meson.build
+++ b/meson.build
@@ -51,6 +51,8 @@
   have_system = have_system or target.endswith('-softmmu')
 endforeach
 have_tools = 'CONFIG_TOOLS' in config_host
+# virtiofsd depends on sysemu
+have_system = have_system or not get_option('virtiofsd').disabled()
 have_block = have_system or have_tools
 
 python = import('python').find_installation()
-- 
2.26.3

[PATCH v2 2/2] gitlab-ci: Add a job to build virtiofsd standalone

[Philippe Mathieu-Daudé]

Add a job which builds virtiofsd without any emulation or tool.

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
https://gitlab.com/philmd/qemu/-/jobs/1222007991
Duration: 7 minutes 48 seconds
---
 .gitlab-ci.yml | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 52d65d6c04f..ba3c7ade6ca 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -800,6 +800,19 @@ build-libvhost-user:
     - meson
     - ninja
 
+build-virtiofsd-fedora:
+  <<: *native_build_job_definition
+  needs:
+    job: amd64-fedora-container
+  variables:
+    IMAGE: fedora
+    CONFIGURE_ARGS: --enable-virtiofsd
+        --disable-system --disable-user --disable-tools --disable-docs
+  artifacts:
+    expire_in: 2 days
+    paths:
+      - build/tools/virtiofsd/virtiofsd
+
 # No targets are built here, just tools, docs, and unit tests. This
 # also feeds into the eventual documentation deployment steps later
 build-tools-and-docs-debian:
-- 
2.26.3

Re: [PATCH v2 2/2] gitlab-ci: Add a job to build virtiofsd standalone

[Daniel P. Berrangé]

On Thu, Apr 29, 2021 at 10:33:46AM +0200, Philippe Mathieu-Daudé wrote:
> Add a job which builds virtiofsd without any emulation or tool.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> https://gitlab.com/philmd/qemu/-/jobs/1222007991
> Duration: 7 minutes 48 seconds
> ---
>  .gitlab-ci.yml | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 52d65d6c04f..ba3c7ade6ca 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -800,6 +800,19 @@ build-libvhost-user:
>      - meson
>      - ninja
>  
> +build-virtiofsd-fedora:
> +  <<: *native_build_job_definition
> +  needs:
> +    job: amd64-fedora-container
> +  variables:
> +    IMAGE: fedora
> +    CONFIGURE_ARGS: --enable-virtiofsd
> +        --disable-system --disable-user --disable-tools --disable-docs
> +  artifacts:
> +    expire_in: 2 days
> +    paths:
> +      - build/tools/virtiofsd/virtiofsd

I'm not convinced that this job is justiable given our need to keep
the total CI pipeline size constrained. The precedent this sets is
that we need to test every configure args combination for each binary
we build. That is not scalable as a pattern. Neither this virtiofsd
arg scenario, nor others is going to be commonly used by downstream
consumers of QEMU, so the payoff from having this job is also small.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v2 1/2] meson: Select 'have_system' when virtiofsd is enabled

[Peter Maydell]

On Thu, 29 Apr 2021 at 09:33, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> When not explicitly select a sysemu target and building virtiofsd,
> the seccomp/cap-ng libraries are not resolved, leading to this error:
>
>   $ configure --target-list=i386-linux-user --disable-tools --enable-virtiofsd
>   tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd requires libcap-ng-devel and seccomp-devel
>
> Fix by enabling sysemu (have_system) when virtiofsd is built.
>
> Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  meson.build | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/meson.build b/meson.build
> index c6f4b0cf5e8..f858935ad95 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -51,6 +51,8 @@
>    have_system = have_system or target.endswith('-softmmu')
>  endforeach
>  have_tools = 'CONFIG_TOOLS' in config_host
> +# virtiofsd depends on sysemu
> +have_system = have_system or not get_option('virtiofsd').disabled()

This looks odd. The natural assumption is that "have_system" ought to mean
"we are building a system emulator", not "we are building a system emulator
or virtiofsd".

thanks
-- PMM

Re: [PATCH v2 2/2] gitlab-ci: Add a job to build virtiofsd standalone

[Philippe Mathieu-Daudé]

On 4/29/21 10:43 AM, Daniel P. Berrangé wrote:
> On Thu, Apr 29, 2021 at 10:33:46AM +0200, Philippe Mathieu-Daudé wrote:
>> Add a job which builds virtiofsd without any emulation or tool.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> https://gitlab.com/philmd/qemu/-/jobs/1222007991
>> Duration: 7 minutes 48 seconds
>> ---
>>  .gitlab-ci.yml | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index 52d65d6c04f..ba3c7ade6ca 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -800,6 +800,19 @@ build-libvhost-user:
>>      - meson
>>      - ninja
>>  
>> +build-virtiofsd-fedora:
>> +  <<: *native_build_job_definition
>> +  needs:
>> +    job: amd64-fedora-container
>> +  variables:
>> +    IMAGE: fedora
>> +    CONFIGURE_ARGS: --enable-virtiofsd
>> +        --disable-system --disable-user --disable-tools --disable-docs
>> +  artifacts:
>> +    expire_in: 2 days
>> +    paths:
>> +      - build/tools/virtiofsd/virtiofsd
> 
> I'm not convinced that this job is justiable given our need to keep
> the total CI pipeline size constrained. The precedent this sets is
> that we need to test every configure args combination for each binary
> we build. That is not scalable as a pattern. Neither this virtiofsd
> arg scenario, nor others is going to be commonly used by downstream
> consumers of QEMU, so the payoff from having this job is also small.

I'm not sure "our current pipelines is too busy because we don't have
a clear idea what is tested and what is duplicated" justifies no more
tests can be added, but it is a effective way to have the current set
cleaned.

Anyhow, if mainstream isn't interested by this configuration, it could
be added to the virtio-fs/qemu fork. Alternatively mainstream with:

  only:
    variables:
      - $CI_PROJECT_NAMESPACE == 'virtio-fs'

Re: [PATCH v2 2/2] gitlab-ci: Add a job to build virtiofsd standalone

[Wainer dos Santos Moschetta]

Hi,

On 4/29/21 5:33 AM, Philippe Mathieu-Daudé wrote:
> Add a job which builds virtiofsd without any emulation or tool.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> https://gitlab.com/philmd/qemu/-/jobs/1222007991
> Duration: 7 minutes 48 seconds
> ---
>   .gitlab-ci.yml | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index 52d65d6c04f..ba3c7ade6ca 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -800,6 +800,19 @@ build-libvhost-user:
>       - meson
>       - ninja
>   
> +build-virtiofsd-fedora:
> +  <<: *native_build_job_definition
> +  needs:
> +    job: amd64-fedora-container
> +  variables:
> +    IMAGE: fedora
> +    CONFIGURE_ARGS: --enable-virtiofsd
> +        --disable-system --disable-user --disable-tools --disable-docs
> +  artifacts:
> +    expire_in: 2 days
> +    paths:
> +      - build/tools/virtiofsd/virtiofsd
> +

Why it needs the executable archived?

- Wainer

>   # No targets are built here, just tools, docs, and unit tests. This
>   # also feeds into the eventual documentation deployment steps later
>   build-tools-and-docs-debian:

Re: [PATCH v2 2/2] gitlab-ci: Add a job to build virtiofsd standalone

[Philippe Mathieu-Daudé]

On 4/29/21 3:16 PM, Wainer dos Santos Moschetta wrote:
> Hi,
> 
> On 4/29/21 5:33 AM, Philippe Mathieu-Daudé wrote:
>> Add a job which builds virtiofsd without any emulation or tool.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>> https://gitlab.com/philmd/qemu/-/jobs/1222007991
>> Duration: 7 minutes 48 seconds
>> ---
>>   .gitlab-ci.yml | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
>> index 52d65d6c04f..ba3c7ade6ca 100644
>> --- a/.gitlab-ci.yml
>> +++ b/.gitlab-ci.yml
>> @@ -800,6 +800,19 @@ build-libvhost-user:
>>       - meson
>>       - ninja
>>   +build-virtiofsd-fedora:
>> +  <<: *native_build_job_definition
>> +  needs:
>> +    job: amd64-fedora-container
>> +  variables:
>> +    IMAGE: fedora
>> +    CONFIGURE_ARGS: --enable-virtiofsd
>> +        --disable-system --disable-user --disable-tools --disable-docs
>> +  artifacts:
>> +    expire_in: 2 days
>> +    paths:
>> +      - build/tools/virtiofsd/virtiofsd
>> +
> 
> Why it needs the executable archived?

I assumed if there were testing jobs as next stage, this is what they
would consume? I copy/pasted from another job and adapted, but you are
right, it is pointless. AnyhowI understood this job has been discarded
during review.

Thanks for having a look!

Phil.

Re: [PATCH v2 1/2] meson: Select 'have_system' when virtiofsd is enabled

[Connor Kuehl]

On 4/29/21 3:33 AM, Philippe Mathieu-Daudé wrote:
> When not explicitly select a sysemu target and building virtiofsd,
> the seccomp/cap-ng libraries are not resolved, leading to this error:
> 
>   $ configure --target-list=i386-linux-user --disable-tools --enable-virtiofsd
>   tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd requires libcap-ng-devel and seccomp-devel
> 
> Fix by enabling sysemu (have_system) when virtiofsd is built.
> 
> Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  meson.build | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index c6f4b0cf5e8..f858935ad95 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -51,6 +51,8 @@
>    have_system = have_system or target.endswith('-softmmu')
>  endforeach
>  have_tools = 'CONFIG_TOOLS' in config_host
> +# virtiofsd depends on sysemu
> +have_system = have_system or not get_option('virtiofsd').disabled()

I don't think we should satisfy virtiofsd dependencies transiently by
depending on system emulation targets.

It's my understanding (and I'm happy to be corrected on this) that the
virtiofsd binary is orthogonal to system emulation tools. Consider a
situation in which someone wants to develop virtiofsd but doesn't want
to wait for the rest of QEMU to build and instead use their own
qemu-system-x86_64 installed by their distro.

Connor

>  have_block = have_system or have_tools
>  
>  python = import('python').find_installation()
> 

Re: [PATCH v2 1/2] meson: Select 'have_system' when virtiofsd is enabled

[Philippe Mathieu-Daudé]

On 4/29/21 3:50 PM, Connor Kuehl wrote:
> On 4/29/21 3:33 AM, Philippe Mathieu-Daudé wrote:
>> When not explicitly select a sysemu target and building virtiofsd,
>> the seccomp/cap-ng libraries are not resolved, leading to this error:
>>
>>   $ configure --target-list=i386-linux-user --disable-tools --enable-virtiofsd
>>   tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd requires libcap-ng-devel and seccomp-devel
>>
>> Fix by enabling sysemu (have_system) when virtiofsd is built.
>>
>> Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  meson.build | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index c6f4b0cf5e8..f858935ad95 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -51,6 +51,8 @@
>>    have_system = have_system or target.endswith('-softmmu')
>>  endforeach
>>  have_tools = 'CONFIG_TOOLS' in config_host
>> +# virtiofsd depends on sysemu
>> +have_system = have_system or not get_option('virtiofsd').disabled()
> 
> I don't think we should satisfy virtiofsd dependencies transiently by
> depending on system emulation targets.
> 
> It's my understanding (and I'm happy to be corrected on this) that the
> virtiofsd binary is orthogonal to system emulation tools. Consider a
> situation in which someone wants to develop virtiofsd but doesn't want
> to wait for the rest of QEMU to build and instead use their own
> qemu-system-x86_64 installed by their distro.

This is what I understood in your previous thread with Mahmoud.

Peter questioned this orthogonality in v1, asking why virtiofsd is
in the QEMU repository then...

> Connor
> 
>>  have_block = have_system or have_tools
>>  
>>  python = import('python').find_installation()
>>
> 

Re: [PATCH v2 1/2] meson: Select 'have_system' when virtiofsd is enabled

[Dr. David Alan Gilbert]

* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> On 4/29/21 3:50 PM, Connor Kuehl wrote:
> > On 4/29/21 3:33 AM, Philippe Mathieu-Daudé wrote:
> >> When not explicitly select a sysemu target and building virtiofsd,
> >> the seccomp/cap-ng libraries are not resolved, leading to this error:
> >>
> >>   $ configure --target-list=i386-linux-user --disable-tools --enable-virtiofsd
> >>   tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd requires libcap-ng-devel and seccomp-devel
> >>
> >> Fix by enabling sysemu (have_system) when virtiofsd is built.
> >>
> >> Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> >> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> ---
> >>  meson.build | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/meson.build b/meson.build
> >> index c6f4b0cf5e8..f858935ad95 100644
> >> --- a/meson.build
> >> +++ b/meson.build
> >> @@ -51,6 +51,8 @@
> >>    have_system = have_system or target.endswith('-softmmu')
> >>  endforeach
> >>  have_tools = 'CONFIG_TOOLS' in config_host
> >> +# virtiofsd depends on sysemu
> >> +have_system = have_system or not get_option('virtiofsd').disabled()
> > 
> > I don't think we should satisfy virtiofsd dependencies transiently by
> > depending on system emulation targets.
> > 
> > It's my understanding (and I'm happy to be corrected on this) that the
> > virtiofsd binary is orthogonal to system emulation tools. Consider a
> > situation in which someone wants to develop virtiofsd but doesn't want
> > to wait for the rest of QEMU to build and instead use their own
> > qemu-system-x86_64 installed by their distro.
> 
> This is what I understood in your previous thread with Mahmoud.
> 
> Peter questioned this orthogonality in v1, asking why virtiofsd is
> in the QEMU repository then...

People were insisting that there was an implementation in when accepting
the qemu side patches.

Dave


> > Connor
> > 
> >>  have_block = have_system or have_tools
> >>  
> >>  python = import('python').find_installation()
> >>
> > 
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v2 1/2] meson: Select 'have_system' when virtiofsd is enabled

[Paolo Bonzini]

On 29/04/21 10:44, Peter Maydell wrote:
> On Thu, 29 Apr 2021 at 09:33, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> When not explicitly select a sysemu target and building virtiofsd,
>> the seccomp/cap-ng libraries are not resolved, leading to this error:
>>
>>    $ configure --target-list=i386-linux-user --disable-tools --enable-virtiofsd
>>    tools/meson.build:12:6: ERROR: Problem encountered: virtiofsd requires libcap-ng-devel and seccomp-devel
>>
>> Fix by enabling sysemu (have_system) when virtiofsd is built.
>>
>> Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   meson.build | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/meson.build b/meson.build
>> index c6f4b0cf5e8..f858935ad95 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -51,6 +51,8 @@
>>     have_system = have_system or target.endswith('-softmmu')
>>   endforeach
>>   have_tools = 'CONFIG_TOOLS' in config_host
>> +# virtiofsd depends on sysemu
>> +have_system = have_system or not get_option('virtiofsd').disabled()
> 
> This looks odd. The natural assumption is that "have_system" ought to mean
> "we are building a system emulator", not "we are building a system emulator
> or virtiofsd".

I agree this is a bit jarring.

If virtiofsd requires libcap-ng-devel and seccomp-devel, those tests 
only should use

   have_system or not get_option('virtiofsd').disabled()

instead of just have_system.  (The reason why we have those complicated 
expressions is to avoid getting warnings on static builds, for libraries 
that are only available as shared libraries.  There are plans to make it 
look a bit nicer in upstream Meson which the maintainers have already 
accepted; however, I haven't been able to contribute much to it lately).

Thanks,

Paolo