All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] virtiofsd: Meson build fixes
@ 2021-04-28 14:48 ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 14:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour, Philippe Mathieu-Daudé

A pair of fixes to allow building virtiofsd without sysemu/tools.

Should this configuration be tested in CI?

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
  util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd

 meson.build      |  4 ++--
 util/meson.build | 10 +++++++---
 2 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.26.3




^ permalink raw reply	[flat|nested] 32+ messages in thread

* [Virtio-fs] [PATCH 0/2] virtiofsd: Meson build fixes
@ 2021-04-28 14:48 ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 14:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour,
	Philippe Mathieu-Daudé

A pair of fixes to allow building virtiofsd without sysemu/tools.

Should this configuration be tested in CI?

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
  util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd

 meson.build      |  4 ++--
 util/meson.build | 10 +++++++---
 2 files changed, 9 insertions(+), 5 deletions(-)

-- 
2.26.3




^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
  2021-04-28 14:48 ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-28 14:48   ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 14:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour, 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 checking the seccomp/cap-ng libraries if virtiofsd is built.

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

diff --git a/meson.build b/meson.build
index c6f4b0cf5e8..b466b418fed 100644
--- a/meson.build
+++ b/meson.build
@@ -393,14 +393,14 @@
 endif
 
 seccomp = not_found
-if not get_option('seccomp').auto() or have_system or have_tools
+if not get_option('seccomp').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
   seccomp = dependency('libseccomp', version: '>=2.3.0',
                        required: get_option('seccomp'),
                        method: 'pkg-config', kwargs: static_kwargs)
 endif
 
 libcap_ng = not_found
-if not get_option('cap_ng').auto() or have_system or have_tools
+if not get_option('cap_ng').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
   libcap_ng = cc.find_library('cap-ng', has_headers: ['cap-ng.h'],
                               required: get_option('cap_ng'),
                               kwargs: static_kwargs)
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Virtio-fs] [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
@ 2021-04-28 14:48   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 14:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour,
	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 checking the seccomp/cap-ng libraries if virtiofsd is built.

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

diff --git a/meson.build b/meson.build
index c6f4b0cf5e8..b466b418fed 100644
--- a/meson.build
+++ b/meson.build
@@ -393,14 +393,14 @@
 endif
 
 seccomp = not_found
-if not get_option('seccomp').auto() or have_system or have_tools
+if not get_option('seccomp').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
   seccomp = dependency('libseccomp', version: '>=2.3.0',
                        required: get_option('seccomp'),
                        method: 'pkg-config', kwargs: static_kwargs)
 endif
 
 libcap_ng = not_found
-if not get_option('cap_ng').auto() or have_system or have_tools
+if not get_option('cap_ng').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
   libcap_ng = cc.find_library('cap-ng', has_headers: ['cap-ng.h'],
                               required: get_option('cap_ng'),
                               kwargs: static_kwargs)
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
  2021-04-28 14:48 ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-28 14:48   ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 14:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour, Philippe Mathieu-Daudé

When not explicitly select a sysemu target or the QEMU tools and
building virtiofsd, libqemuutil.a has missing symbols:

  /usr/bin/ld: tools/virtiofsd/virtiofsd.p/fuse_virtio.c.o: in function `virtio_send_msg':
  tools/virtiofsd/fuse_virtio.c:236: undefined reference to `iov_size'

  /usr/bin/ld: libqemuutil.a(util_iov.c.o): in function `iov_hexdump':
  util/iov.c:240: undefined reference to `qemu_hexdump'

  /usr/bin/ld: libqemuutil.a(util_iov.c.o): in function `qemu_iovec_is_zero':
  util/iov.c:494: undefined reference to `buffer_is_zero'

Fix by linking bufferiszero/hexdump/iov objects when building
virtiofsd (regardless of sysemu / tools).

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 util/meson.build | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/util/meson.build b/util/meson.build
index 510765cde46..c2eda2d1374 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -59,12 +59,10 @@
   util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
   util_ss.add(files('base64.c'))
   util_ss.add(files('buffer.c'))
-  util_ss.add(files('bufferiszero.c'))
   util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
   util_ss.add(files('hbitmap.c'))
-  util_ss.add(files('hexdump.c'))
   util_ss.add(files('iova-tree.c'))
-  util_ss.add(files('iov.c', 'qemu-sockets.c', 'uri.c'))
+  util_ss.add(files('qemu-sockets.c', 'uri.c'))
   util_ss.add(files('lockcnt.c'))
   util_ss.add(files('main-loop.c'))
   util_ss.add(files('nvdimm-utils.c'))
@@ -83,3 +81,9 @@
                                         if_false: files('filemonitor-stub.c'))
   util_ss.add(when: 'CONFIG_LINUX', if_true: files('vfio-helpers.c'))
 endif
+
+if have_block or config_host.has_key('CONFIG_VHOST_USER_FS')
+  util_ss.add(files('hexdump.c'))
+  util_ss.add(files('bufferiszero.c'))
+  util_ss.add(files('iov.c'))
+endif
-- 
2.26.3



^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [Virtio-fs] [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
@ 2021-04-28 14:48   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 14:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour,
	Philippe Mathieu-Daudé

When not explicitly select a sysemu target or the QEMU tools and
building virtiofsd, libqemuutil.a has missing symbols:

  /usr/bin/ld: tools/virtiofsd/virtiofsd.p/fuse_virtio.c.o: in function `virtio_send_msg':
  tools/virtiofsd/fuse_virtio.c:236: undefined reference to `iov_size'

  /usr/bin/ld: libqemuutil.a(util_iov.c.o): in function `iov_hexdump':
  util/iov.c:240: undefined reference to `qemu_hexdump'

  /usr/bin/ld: libqemuutil.a(util_iov.c.o): in function `qemu_iovec_is_zero':
  util/iov.c:494: undefined reference to `buffer_is_zero'

Fix by linking bufferiszero/hexdump/iov objects when building
virtiofsd (regardless of sysemu / tools).

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 util/meson.build | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/util/meson.build b/util/meson.build
index 510765cde46..c2eda2d1374 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -59,12 +59,10 @@
   util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
   util_ss.add(files('base64.c'))
   util_ss.add(files('buffer.c'))
-  util_ss.add(files('bufferiszero.c'))
   util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
   util_ss.add(files('hbitmap.c'))
-  util_ss.add(files('hexdump.c'))
   util_ss.add(files('iova-tree.c'))
-  util_ss.add(files('iov.c', 'qemu-sockets.c', 'uri.c'))
+  util_ss.add(files('qemu-sockets.c', 'uri.c'))
   util_ss.add(files('lockcnt.c'))
   util_ss.add(files('main-loop.c'))
   util_ss.add(files('nvdimm-utils.c'))
@@ -83,3 +81,9 @@
                                         if_false: files('filemonitor-stub.c'))
   util_ss.add(when: 'CONFIG_LINUX', if_true: files('vfio-helpers.c'))
 endif
+
+if have_block or config_host.has_key('CONFIG_VHOST_USER_FS')
+  util_ss.add(files('hexdump.c'))
+  util_ss.add(files('bufferiszero.c'))
+  util_ss.add(files('iov.c'))
+endif
-- 
2.26.3


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
  2021-04-28 14:48   ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-28 14:52     ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 14:52 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour

On 4/28/21 4:48 PM, Philippe Mathieu-Daudé wrote:
> When not explicitly select a sysemu target or the QEMU tools and
> building virtiofsd, libqemuutil.a has missing symbols:
> 
>   /usr/bin/ld: tools/virtiofsd/virtiofsd.p/fuse_virtio.c.o: in function `virtio_send_msg':
>   tools/virtiofsd/fuse_virtio.c:236: undefined reference to `iov_size'
> 
>   /usr/bin/ld: libqemuutil.a(util_iov.c.o): in function `iov_hexdump':
>   util/iov.c:240: undefined reference to `qemu_hexdump'
> 
>   /usr/bin/ld: libqemuutil.a(util_iov.c.o): in function `qemu_iovec_is_zero':
>   util/iov.c:494: undefined reference to `buffer_is_zero'
> 
> Fix by linking bufferiszero/hexdump/iov objects when building
> virtiofsd (regardless of sysemu / tools).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  util/meson.build | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/util/meson.build b/util/meson.build
> index 510765cde46..c2eda2d1374 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -59,12 +59,10 @@
>    util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
>    util_ss.add(files('base64.c'))
>    util_ss.add(files('buffer.c'))
> -  util_ss.add(files('bufferiszero.c'))
>    util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
>    util_ss.add(files('hbitmap.c'))
> -  util_ss.add(files('hexdump.c'))
>    util_ss.add(files('iova-tree.c'))
> -  util_ss.add(files('iov.c', 'qemu-sockets.c', 'uri.c'))
> +  util_ss.add(files('qemu-sockets.c', 'uri.c'))
>    util_ss.add(files('lockcnt.c'))
>    util_ss.add(files('main-loop.c'))
>    util_ss.add(files('nvdimm-utils.c'))
> @@ -83,3 +81,9 @@
>                                          if_false: files('filemonitor-stub.c'))
>    util_ss.add(when: 'CONFIG_LINUX', if_true: files('vfio-helpers.c'))
>  endif
> +
> +if have_block or config_host.has_key('CONFIG_VHOST_USER_FS')
> +  util_ss.add(files('hexdump.c'))
> +  util_ss.add(files('bufferiszero.c'))

Doh it isn't sorted :/ and maybe better subject:
"Build virtiofsd with iov/hexdump/buffer_is_zero objects"?

> +  util_ss.add(files('iov.c'))
> +endif
> 



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
@ 2021-04-28 14:52     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 14:52 UTC (permalink / raw)
  To: qemu-devel; +Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour

On 4/28/21 4:48 PM, Philippe Mathieu-Daudé wrote:
> When not explicitly select a sysemu target or the QEMU tools and
> building virtiofsd, libqemuutil.a has missing symbols:
> 
>   /usr/bin/ld: tools/virtiofsd/virtiofsd.p/fuse_virtio.c.o: in function `virtio_send_msg':
>   tools/virtiofsd/fuse_virtio.c:236: undefined reference to `iov_size'
> 
>   /usr/bin/ld: libqemuutil.a(util_iov.c.o): in function `iov_hexdump':
>   util/iov.c:240: undefined reference to `qemu_hexdump'
> 
>   /usr/bin/ld: libqemuutil.a(util_iov.c.o): in function `qemu_iovec_is_zero':
>   util/iov.c:494: undefined reference to `buffer_is_zero'
> 
> Fix by linking bufferiszero/hexdump/iov objects when building
> virtiofsd (regardless of sysemu / tools).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  util/meson.build | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/util/meson.build b/util/meson.build
> index 510765cde46..c2eda2d1374 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -59,12 +59,10 @@
>    util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
>    util_ss.add(files('base64.c'))
>    util_ss.add(files('buffer.c'))
> -  util_ss.add(files('bufferiszero.c'))
>    util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
>    util_ss.add(files('hbitmap.c'))
> -  util_ss.add(files('hexdump.c'))
>    util_ss.add(files('iova-tree.c'))
> -  util_ss.add(files('iov.c', 'qemu-sockets.c', 'uri.c'))
> +  util_ss.add(files('qemu-sockets.c', 'uri.c'))
>    util_ss.add(files('lockcnt.c'))
>    util_ss.add(files('main-loop.c'))
>    util_ss.add(files('nvdimm-utils.c'))
> @@ -83,3 +81,9 @@
>                                          if_false: files('filemonitor-stub.c'))
>    util_ss.add(when: 'CONFIG_LINUX', if_true: files('vfio-helpers.c'))
>  endif
> +
> +if have_block or config_host.has_key('CONFIG_VHOST_USER_FS')
> +  util_ss.add(files('hexdump.c'))
> +  util_ss.add(files('bufferiszero.c'))

Doh it isn't sorted :/ and maybe better subject:
"Build virtiofsd with iov/hexdump/buffer_is_zero objects"?

> +  util_ss.add(files('iov.c'))
> +endif
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
  2021-04-28 14:48   ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-28 15:06     ` Peter Maydell
  -1 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2021-04-28 15:06 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl, QEMU Developers,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Paolo Bonzini, Mahmoud Mandour

On Wed, 28 Apr 2021 at 15:55, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

(cc'ing Paolo for a meson.build question below...)

> 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 checking the seccomp/cap-ng libraries if virtiofsd is built.
>
> Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  meson.build | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index c6f4b0cf5e8..b466b418fed 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -393,14 +393,14 @@
>  endif
>
>  seccomp = not_found
> -if not get_option('seccomp').auto() or have_system or have_tools
> +if not get_option('seccomp').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>    seccomp = dependency('libseccomp', version: '>=2.3.0',
>                         required: get_option('seccomp'),
>                         method: 'pkg-config', kwargs: static_kwargs)
>  endif
>
>  libcap_ng = not_found
> -if not get_option('cap_ng').auto() or have_system or have_tools
> +if not get_option('cap_ng').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>    libcap_ng = cc.find_library('cap-ng', has_headers: ['cap-ng.h'],
>                                required: get_option('cap_ng'),
>                                kwargs: static_kwargs)

Now we have "virtiofsd requires cap-ng and seccomp" recorded in three
places in different meson.build files:
 * here, if this patch goes in
 * in tools/meson.build, in its logic for setting have_virtiofsd
   (I generously do not count the "decide which error message to print"
   logic in that file as a separate item in this list...)
 * in tools/virtiofsd/meson.build, where the executable('virtiofsd', ...)
   lists them in its dependencies: setting

Is there some way to avoid this duplication?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
@ 2021-04-28 15:06     ` Peter Maydell
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2021-04-28 15:06 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Daniel P . Berrange, QEMU Developers, virtio-fs,
	Paolo Bonzini, Mahmoud Mandour

On Wed, 28 Apr 2021 at 15:55, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

(cc'ing Paolo for a meson.build question below...)

> 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 checking the seccomp/cap-ng libraries if virtiofsd is built.
>
> Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  meson.build | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index c6f4b0cf5e8..b466b418fed 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -393,14 +393,14 @@
>  endif
>
>  seccomp = not_found
> -if not get_option('seccomp').auto() or have_system or have_tools
> +if not get_option('seccomp').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>    seccomp = dependency('libseccomp', version: '>=2.3.0',
>                         required: get_option('seccomp'),
>                         method: 'pkg-config', kwargs: static_kwargs)
>  endif
>
>  libcap_ng = not_found
> -if not get_option('cap_ng').auto() or have_system or have_tools
> +if not get_option('cap_ng').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>    libcap_ng = cc.find_library('cap-ng', has_headers: ['cap-ng.h'],
>                                required: get_option('cap_ng'),
>                                kwargs: static_kwargs)

Now we have "virtiofsd requires cap-ng and seccomp" recorded in three
places in different meson.build files:
 * here, if this patch goes in
 * in tools/meson.build, in its logic for setting have_virtiofsd
   (I generously do not count the "decide which error message to print"
   logic in that file as a separate item in this list...)
 * in tools/virtiofsd/meson.build, where the executable('virtiofsd', ...)
   lists them in its dependencies: setting

Is there some way to avoid this duplication?

thanks
-- PMM



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/2] virtiofsd: Meson build fixes
  2021-04-28 14:48 ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-28 15:15   ` Peter Maydell
  -1 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2021-04-28 15:15 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl, QEMU Developers,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour

On Wed, 28 Apr 2021 at 15:53, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> A pair of fixes to allow building virtiofsd without sysemu/tools.
>
> Should this configuration be tested in CI?

Does the configuration make sense to support? I thought
virtiofsd was only of use with system emulation ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 0/2] virtiofsd: Meson build fixes
@ 2021-04-28 15:15   ` Peter Maydell
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2021-04-28 15:15 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Daniel P . Berrange, QEMU Developers, virtio-fs,
	Mahmoud Mandour

On Wed, 28 Apr 2021 at 15:53, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> A pair of fixes to allow building virtiofsd without sysemu/tools.
>
> Should this configuration be tested in CI?

Does the configuration make sense to support? I thought
virtiofsd was only of use with system emulation ?

thanks
-- PMM



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
  2021-04-28 15:06     ` [Virtio-fs] " Peter Maydell
@ 2021-04-28 15:34       ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 15:34 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl, QEMU Developers,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Paolo Bonzini, Mahmoud Mandour

On 4/28/21 5:06 PM, Peter Maydell wrote:
> On Wed, 28 Apr 2021 at 15:55, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
> (cc'ing Paolo for a meson.build question below...)
> 
>> 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 checking the seccomp/cap-ng libraries if virtiofsd is built.
>>
>> Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  meson.build | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index c6f4b0cf5e8..b466b418fed 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -393,14 +393,14 @@
>>  endif
>>
>>  seccomp = not_found
>> -if not get_option('seccomp').auto() or have_system or have_tools
>> +if not get_option('seccomp').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>>    seccomp = dependency('libseccomp', version: '>=2.3.0',
>>                         required: get_option('seccomp'),
>>                         method: 'pkg-config', kwargs: static_kwargs)
>>  endif
>>
>>  libcap_ng = not_found
>> -if not get_option('cap_ng').auto() or have_system or have_tools
>> +if not get_option('cap_ng').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>>    libcap_ng = cc.find_library('cap-ng', has_headers: ['cap-ng.h'],
>>                                required: get_option('cap_ng'),
>>                                kwargs: static_kwargs)
> 
> Now we have "virtiofsd requires cap-ng and seccomp" recorded in three
> places in different meson.build files:
>  * here, if this patch goes in
>  * in tools/meson.build, in its logic for setting have_virtiofsd
>    (I generously do not count the "decide which error message to print"
>    logic in that file as a separate item in this list...)
>  * in tools/virtiofsd/meson.build, where the executable('virtiofsd', ...)
>    lists them in its dependencies: setting
> 
> Is there some way to avoid this duplication?

I noticed that too, I think the problem is we never considered the case
of a standalone binary buildable without sysemu / tools, such virtiofsd.

TIL virtiofsd is not a tool, so tools/meson.build needs rework, in
particular to remove the 'have_tools' but maybe to move it out of
tools/:

have_virtiofsd = (targetos == 'linux' and
    have_tools and
    seccomp.found() and
    libcap_ng.found() and
    'CONFIG_VHOST_USER' in config_host)

If virtiofsd requires Linux, shouldn't we check it directly
in ./configure, not allowing --enable-virtiofsd on non-linux
hosts?
Maybe it is there for cross-compilation... Is it supported to
cross-build it on non-Linux hosts?


It could be clearer to use:

  want_virtiofsd = get_option('virtiofsd').auto()


BTW to test I used:

  $ configure --target-list=i386-linux-user --disable-tools
--enable-virtiofsd
  $ ninja tools/virtiofsd/virtiofsd



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
@ 2021-04-28 15:34       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 15:34 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, Daniel P . Berrange, QEMU Developers, virtio-fs,
	Paolo Bonzini, Mahmoud Mandour

On 4/28/21 5:06 PM, Peter Maydell wrote:
> On Wed, 28 Apr 2021 at 15:55, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> 
> (cc'ing Paolo for a meson.build question below...)
> 
>> 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 checking the seccomp/cap-ng libraries if virtiofsd is built.
>>
>> Reported-by: Mahmoud Mandour <ma.mandourr@gmail.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  meson.build | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/meson.build b/meson.build
>> index c6f4b0cf5e8..b466b418fed 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -393,14 +393,14 @@
>>  endif
>>
>>  seccomp = not_found
>> -if not get_option('seccomp').auto() or have_system or have_tools
>> +if not get_option('seccomp').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>>    seccomp = dependency('libseccomp', version: '>=2.3.0',
>>                         required: get_option('seccomp'),
>>                         method: 'pkg-config', kwargs: static_kwargs)
>>  endif
>>
>>  libcap_ng = not_found
>> -if not get_option('cap_ng').auto() or have_system or have_tools
>> +if not get_option('cap_ng').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>>    libcap_ng = cc.find_library('cap-ng', has_headers: ['cap-ng.h'],
>>                                required: get_option('cap_ng'),
>>                                kwargs: static_kwargs)
> 
> Now we have "virtiofsd requires cap-ng and seccomp" recorded in three
> places in different meson.build files:
>  * here, if this patch goes in
>  * in tools/meson.build, in its logic for setting have_virtiofsd
>    (I generously do not count the "decide which error message to print"
>    logic in that file as a separate item in this list...)
>  * in tools/virtiofsd/meson.build, where the executable('virtiofsd', ...)
>    lists them in its dependencies: setting
> 
> Is there some way to avoid this duplication?

I noticed that too, I think the problem is we never considered the case
of a standalone binary buildable without sysemu / tools, such virtiofsd.

TIL virtiofsd is not a tool, so tools/meson.build needs rework, in
particular to remove the 'have_tools' but maybe to move it out of
tools/:

have_virtiofsd = (targetos == 'linux' and
    have_tools and
    seccomp.found() and
    libcap_ng.found() and
    'CONFIG_VHOST_USER' in config_host)

If virtiofsd requires Linux, shouldn't we check it directly
in ./configure, not allowing --enable-virtiofsd on non-linux
hosts?
Maybe it is there for cross-compilation... Is it supported to
cross-build it on non-Linux hosts?


It could be clearer to use:

  want_virtiofsd = get_option('virtiofsd').auto()


BTW to test I used:

  $ configure --target-list=i386-linux-user --disable-tools
--enable-virtiofsd
  $ ninja tools/virtiofsd/virtiofsd


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
  2021-04-28 14:48   ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-28 16:34     ` Richard Henderson
  -1 siblings, 0 replies; 32+ messages in thread
From: Richard Henderson @ 2021-04-28 16:34 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour

On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>   seccomp = not_found
> -if not get_option('seccomp').auto() or have_system or have_tools
> +if not get_option('seccomp').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>     seccomp = dependency('libseccomp', version: '>=2.3.0',
>                          required: get_option('seccomp'),
>                          method: 'pkg-config', kwargs: static_kwargs)

This construct is wrong, both before and after, as I read it.

not get_option(foo).auto() is true for both enabled and disabled.  If disabled, 
why are we examining the dependency?  If auto, if we have all of the 
dependencies we want to enable the feature -- if we don't probe for the 
dependency, how can we enable it?

This error seems to be offset by the OR have_* tests, for which the logic also 
seems off.

I think the test should have been

   if (have_system or have_tools) and
      (not get_option('seccomp').disabled() or
       not get_option('virtiofsd').disabled())

Then we need to combine the required: argument, probably like

   required: get_option('seccomp').enabled() or
             get_option('virtiofsd').enabled()


r~


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
@ 2021-04-28 16:34     ` Richard Henderson
  0 siblings, 0 replies; 32+ messages in thread
From: Richard Henderson @ 2021-04-28 16:34 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour

On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>   seccomp = not_found
> -if not get_option('seccomp').auto() or have_system or have_tools
> +if not get_option('seccomp').auto() or have_system or have_tools or not get_option('virtiofsd').auto()
>     seccomp = dependency('libseccomp', version: '>=2.3.0',
>                          required: get_option('seccomp'),
>                          method: 'pkg-config', kwargs: static_kwargs)

This construct is wrong, both before and after, as I read it.

not get_option(foo).auto() is true for both enabled and disabled.  If disabled, 
why are we examining the dependency?  If auto, if we have all of the 
dependencies we want to enable the feature -- if we don't probe for the 
dependency, how can we enable it?

This error seems to be offset by the OR have_* tests, for which the logic also 
seems off.

I think the test should have been

   if (have_system or have_tools) and
      (not get_option('seccomp').disabled() or
       not get_option('virtiofsd').disabled())

Then we need to combine the required: argument, probably like

   required: get_option('seccomp').enabled() or
             get_option('virtiofsd').enabled()


r~


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
  2021-04-28 14:48   ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-28 16:38     ` Richard Henderson
  -1 siblings, 0 replies; 32+ messages in thread
From: Richard Henderson @ 2021-04-28 16:38 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour

On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
> diff --git a/util/meson.build b/util/meson.build
> index 510765cde46..c2eda2d1374 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -59,12 +59,10 @@
>     util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
>     util_ss.add(files('base64.c'))
>     util_ss.add(files('buffer.c'))
> -  util_ss.add(files('bufferiszero.c'))
>     util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
>     util_ss.add(files('hbitmap.c'))
> -  util_ss.add(files('hexdump.c'))
>     util_ss.add(files('iova-tree.c'))
> -  util_ss.add(files('iov.c', 'qemu-sockets.c', 'uri.c'))
> +  util_ss.add(files('qemu-sockets.c', 'uri.c'))
>     util_ss.add(files('lockcnt.c'))
>     util_ss.add(files('main-loop.c'))
>     util_ss.add(files('nvdimm-utils.c'))
> @@ -83,3 +81,9 @@
>                                           if_false: files('filemonitor-stub.c'))
>     util_ss.add(when: 'CONFIG_LINUX', if_true: files('vfio-helpers.c'))
>   endif
> +
> +if have_block or config_host.has_key('CONFIG_VHOST_USER_FS')
> +  util_ss.add(files('hexdump.c'))
> +  util_ss.add(files('bufferiszero.c'))
> +  util_ss.add(files('iov.c'))
> +endif

Isn't util a static library, built once?  Why are we avoiding building these 
unconditionally?  Surely symbols will be included in any linked binaries only 
as needed.


r~


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
@ 2021-04-28 16:38     ` Richard Henderson
  0 siblings, 0 replies; 32+ messages in thread
From: Richard Henderson @ 2021-04-28 16:38 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour

On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
> diff --git a/util/meson.build b/util/meson.build
> index 510765cde46..c2eda2d1374 100644
> --- a/util/meson.build
> +++ b/util/meson.build
> @@ -59,12 +59,10 @@
>     util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
>     util_ss.add(files('base64.c'))
>     util_ss.add(files('buffer.c'))
> -  util_ss.add(files('bufferiszero.c'))
>     util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
>     util_ss.add(files('hbitmap.c'))
> -  util_ss.add(files('hexdump.c'))
>     util_ss.add(files('iova-tree.c'))
> -  util_ss.add(files('iov.c', 'qemu-sockets.c', 'uri.c'))
> +  util_ss.add(files('qemu-sockets.c', 'uri.c'))
>     util_ss.add(files('lockcnt.c'))
>     util_ss.add(files('main-loop.c'))
>     util_ss.add(files('nvdimm-utils.c'))
> @@ -83,3 +81,9 @@
>                                           if_false: files('filemonitor-stub.c'))
>     util_ss.add(when: 'CONFIG_LINUX', if_true: files('vfio-helpers.c'))
>   endif
> +
> +if have_block or config_host.has_key('CONFIG_VHOST_USER_FS')
> +  util_ss.add(files('hexdump.c'))
> +  util_ss.add(files('bufferiszero.c'))
> +  util_ss.add(files('iov.c'))
> +endif

Isn't util a static library, built once?  Why are we avoiding building these 
unconditionally?  Surely symbols will be included in any linked binaries only 
as needed.


r~


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 0/2] virtiofsd: Meson build fixes
  2021-04-28 15:15   ` [Virtio-fs] " Peter Maydell
@ 2021-04-28 16:52     ` Dr. David Alan Gilbert
  -1 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-28 16:52 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl, QEMU Developers,
	virtio-fs, Stefan Hajnoczi, Mahmoud Mandour,
	Philippe Mathieu-Daudé

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Wed, 28 Apr 2021 at 15:53, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >
> > A pair of fixes to allow building virtiofsd without sysemu/tools.
> >
> > Should this configuration be tested in CI?
> 
> Does the configuration make sense to support? I thought
> virtiofsd was only of use with system emulation ?

It probably is; although arguably it makes sense to build
virtiofsd by itself without either emulation.

Dave

> thanks
> -- PMM
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 0/2] virtiofsd: Meson build fixes
@ 2021-04-28 16:52     ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 32+ messages in thread
From: Dr. David Alan Gilbert @ 2021-04-28 16:52 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Thomas Huth, Daniel P . Berrange, QEMU Developers, virtio-fs,
	Mahmoud Mandour, Philippe Mathieu-Daudé

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On Wed, 28 Apr 2021 at 15:53, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >
> > A pair of fixes to allow building virtiofsd without sysemu/tools.
> >
> > Should this configuration be tested in CI?
> 
> Does the configuration make sense to support? I thought
> virtiofsd was only of use with system emulation ?

It probably is; although arguably it makes sense to build
virtiofsd by itself without either emulation.

Dave

> thanks
> -- PMM
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
  2021-04-28 16:38     ` [Virtio-fs] " Richard Henderson
@ 2021-04-28 17:56       ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 17:56 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour

On 4/28/21 6:38 PM, Richard Henderson wrote:
> On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>> diff --git a/util/meson.build b/util/meson.build
>> index 510765cde46..c2eda2d1374 100644
>> --- a/util/meson.build
>> +++ b/util/meson.build
>> @@ -59,12 +59,10 @@
>>     util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
>>     util_ss.add(files('base64.c'))
>>     util_ss.add(files('buffer.c'))
>> -  util_ss.add(files('bufferiszero.c'))
>>    
>> util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
>>
>>     util_ss.add(files('hbitmap.c'))
>> -  util_ss.add(files('hexdump.c'))
>>     util_ss.add(files('iova-tree.c'))
>> -  util_ss.add(files('iov.c', 'qemu-sockets.c', 'uri.c'))
>> +  util_ss.add(files('qemu-sockets.c', 'uri.c'))
>>     util_ss.add(files('lockcnt.c'))
>>     util_ss.add(files('main-loop.c'))
>>     util_ss.add(files('nvdimm-utils.c'))
>> @@ -83,3 +81,9 @@
>>                                           if_false:
>> files('filemonitor-stub.c'))
>>     util_ss.add(when: 'CONFIG_LINUX', if_true: files('vfio-helpers.c'))
>>   endif
>> +
>> +if have_block or config_host.has_key('CONFIG_VHOST_USER_FS')
>> +  util_ss.add(files('hexdump.c'))
>> +  util_ss.add(files('bufferiszero.c'))
>> +  util_ss.add(files('iov.c'))
>> +endif
> 
> Isn't util a static library, built once?  Why are we avoiding building
> these unconditionally?  Surely symbols will be included in any linked
> binaries only as needed.

Yes, in this case built once for $ configure
--target-list=i386-linux-user --disable-tools --enable-virtiofsd.

Are you suggesting to remove the 'if have_block' check? This makes build
a the files pointlessly for user-mode-only builds...



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
@ 2021-04-28 17:56       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 17:56 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour

On 4/28/21 6:38 PM, Richard Henderson wrote:
> On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>> diff --git a/util/meson.build b/util/meson.build
>> index 510765cde46..c2eda2d1374 100644
>> --- a/util/meson.build
>> +++ b/util/meson.build
>> @@ -59,12 +59,10 @@
>>     util_ss.add(files('aiocb.c', 'async.c', 'aio-wait.c'))
>>     util_ss.add(files('base64.c'))
>>     util_ss.add(files('buffer.c'))
>> -  util_ss.add(files('bufferiszero.c'))
>>    
>> util_ss.add(files('coroutine-@0@.c'.format(config_host['CONFIG_COROUTINE_BACKEND'])))
>>
>>     util_ss.add(files('hbitmap.c'))
>> -  util_ss.add(files('hexdump.c'))
>>     util_ss.add(files('iova-tree.c'))
>> -  util_ss.add(files('iov.c', 'qemu-sockets.c', 'uri.c'))
>> +  util_ss.add(files('qemu-sockets.c', 'uri.c'))
>>     util_ss.add(files('lockcnt.c'))
>>     util_ss.add(files('main-loop.c'))
>>     util_ss.add(files('nvdimm-utils.c'))
>> @@ -83,3 +81,9 @@
>>                                           if_false:
>> files('filemonitor-stub.c'))
>>     util_ss.add(when: 'CONFIG_LINUX', if_true: files('vfio-helpers.c'))
>>   endif
>> +
>> +if have_block or config_host.has_key('CONFIG_VHOST_USER_FS')
>> +  util_ss.add(files('hexdump.c'))
>> +  util_ss.add(files('bufferiszero.c'))
>> +  util_ss.add(files('iov.c'))
>> +endif
> 
> Isn't util a static library, built once?  Why are we avoiding building
> these unconditionally?  Surely symbols will be included in any linked
> binaries only as needed.

Yes, in this case built once for $ configure
--target-list=i386-linux-user --disable-tools --enable-virtiofsd.

Are you suggesting to remove the 'if have_block' check? This makes build
a the files pointlessly for user-mode-only builds...


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
  2021-04-28 16:34     ` [Virtio-fs] " Richard Henderson
@ 2021-04-28 18:00       ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 18:00 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, Paolo Bonzini
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour

On 4/28/21 6:34 PM, Richard Henderson wrote:
> On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>>   seccomp = not_found
>> -if not get_option('seccomp').auto() or have_system or have_tools
>> +if not get_option('seccomp').auto() or have_system or have_tools or
>> not get_option('virtiofsd').auto()
>>     seccomp = dependency('libseccomp', version: '>=2.3.0',
>>                          required: get_option('seccomp'),
>>                          method: 'pkg-config', kwargs: static_kwargs)
> 
> This construct is wrong, both before and after, as I read it.
> 
> not get_option(foo).auto() is true for both enabled and disabled.  If
> disabled, why are we examining the dependency?  If auto, if we have all
> of the dependencies we want to enable the feature -- if we don't probe
> for the dependency, how can we enable it?
> 
> This error seems to be offset by the OR have_* tests, for which the
> logic also seems off.
> 
> I think the test should have been
> 
>   if (have_system or have_tools) and

Yes but virtiofsd is not a tool... It is a standalone binary.
Maybe have_system is the culprit here:

  have_system = have_system or target.endswith('-softmmu')

We should somewhere add:

  have_system = have_system or something('virtiofsd')

However I wonder if we aren't going to build many objects
that are irrelevant for virtiofsd.

>      (not get_option('seccomp').disabled() or
>       not get_option('virtiofsd').disabled())
> 
> Then we need to combine the required: argument, probably like
> 
>   required: get_option('seccomp').enabled() or
>             get_option('virtiofsd').enabled()
> 
> 
> r~
> 



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
@ 2021-04-28 18:00       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 18:00 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, Paolo Bonzini
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour

On 4/28/21 6:34 PM, Richard Henderson wrote:
> On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>>   seccomp = not_found
>> -if not get_option('seccomp').auto() or have_system or have_tools
>> +if not get_option('seccomp').auto() or have_system or have_tools or
>> not get_option('virtiofsd').auto()
>>     seccomp = dependency('libseccomp', version: '>=2.3.0',
>>                          required: get_option('seccomp'),
>>                          method: 'pkg-config', kwargs: static_kwargs)
> 
> This construct is wrong, both before and after, as I read it.
> 
> not get_option(foo).auto() is true for both enabled and disabled.  If
> disabled, why are we examining the dependency?  If auto, if we have all
> of the dependencies we want to enable the feature -- if we don't probe
> for the dependency, how can we enable it?
> 
> This error seems to be offset by the OR have_* tests, for which the
> logic also seems off.
> 
> I think the test should have been
> 
>   if (have_system or have_tools) and

Yes but virtiofsd is not a tool... It is a standalone binary.
Maybe have_system is the culprit here:

  have_system = have_system or target.endswith('-softmmu')

We should somewhere add:

  have_system = have_system or something('virtiofsd')

However I wonder if we aren't going to build many objects
that are irrelevant for virtiofsd.

>      (not get_option('seccomp').disabled() or
>       not get_option('virtiofsd').disabled())
> 
> Then we need to combine the required: argument, probably like
> 
>   required: get_option('seccomp').enabled() or
>             get_option('virtiofsd').enabled()
> 
> 
> r~
> 


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
  2021-04-28 18:00       ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-28 18:36         ` Peter Maydell
  -1 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2021-04-28 18:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Richard Henderson, QEMU Developers, Dr. David Alan Gilbert,
	virtio-fs, Stefan Hajnoczi, Paolo Bonzini, Mahmoud Mandour

On Wed, 28 Apr 2021 at 19:02, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 4/28/21 6:34 PM, Richard Henderson wrote:
> > I think the test should have been
> >
> >   if (have_system or have_tools) and
>
> Yes but virtiofsd is not a tool... It is a standalone binary.

This is not a distinction that our build/configure system
currently makes. We have three categories:
 * user-mode emulator binaries
 * system-mode emulator binaries
 * tools

where essentially a "tool" is "any binary we build that isn't
a QEMU binary proper".

If virtiofsd is genuinely not related to QEMU at all, what is
it doing in our source tree ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
@ 2021-04-28 18:36         ` Peter Maydell
  0 siblings, 0 replies; 32+ messages in thread
From: Peter Maydell @ 2021-04-28 18:36 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Thomas Huth, Daniel P . Berrange, Richard Henderson,
	QEMU Developers, virtio-fs, Paolo Bonzini, Mahmoud Mandour

On Wed, 28 Apr 2021 at 19:02, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 4/28/21 6:34 PM, Richard Henderson wrote:
> > I think the test should have been
> >
> >   if (have_system or have_tools) and
>
> Yes but virtiofsd is not a tool... It is a standalone binary.

This is not a distinction that our build/configure system
currently makes. We have three categories:
 * user-mode emulator binaries
 * system-mode emulator binaries
 * tools

where essentially a "tool" is "any binary we build that isn't
a QEMU binary proper".

If virtiofsd is genuinely not related to QEMU at all, what is
it doing in our source tree ?

thanks
-- PMM



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
  2021-04-28 17:56       ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-28 19:24         ` Richard Henderson
  -1 siblings, 0 replies; 32+ messages in thread
From: Richard Henderson @ 2021-04-28 19:24 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour

On 4/28/21 10:56 AM, Philippe Mathieu-Daudé wrote:
> Are you suggesting to remove the 'if have_block' check? This makes build
> a the files pointlessly for user-mode-only builds...

But since the objects are not included in the binary, I don't care.

The build system is already too complex, and building a couple of extra small 
files makes only milliseconds of difference.


r~


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
@ 2021-04-28 19:24         ` Richard Henderson
  0 siblings, 0 replies; 32+ messages in thread
From: Richard Henderson @ 2021-04-28 19:24 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour

On 4/28/21 10:56 AM, Philippe Mathieu-Daudé wrote:
> Are you suggesting to remove the 'if have_block' check? This makes build
> a the files pointlessly for user-mode-only builds...

But since the objects are not included in the binary, I don't care.

The build system is already too complex, and building a couple of extra small 
files makes only milliseconds of difference.


r~


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
  2021-04-28 19:24         ` [Virtio-fs] " Richard Henderson
@ 2021-04-28 19:34           ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 19:34 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour

On 4/28/21 9:24 PM, Richard Henderson wrote:
> On 4/28/21 10:56 AM, Philippe Mathieu-Daudé wrote:
>> Are you suggesting to remove the 'if have_block' check? This makes build
>> a the files pointlessly for user-mode-only builds...
> 
> But since the objects are not included in the binary, I don't care.
> 
> The build system is already too complex, and building a couple of extra
> small files makes only milliseconds of difference.

Maybe for libqemuutil.a (this does make a difference with the Python
QAPI generated files - another series).

I'll wait if we get to a consensus about what exactly is virtiofsd,
then revisit this series.

Thanks!

Phil.



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd
@ 2021-04-28 19:34           ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-28 19:34 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour

On 4/28/21 9:24 PM, Richard Henderson wrote:
> On 4/28/21 10:56 AM, Philippe Mathieu-Daudé wrote:
>> Are you suggesting to remove the 'if have_block' check? This makes build
>> a the files pointlessly for user-mode-only builds...
> 
> But since the objects are not included in the binary, I don't care.
> 
> The build system is already too complex, and building a couple of extra
> small files makes only milliseconds of difference.

Maybe for libqemuutil.a (this does make a difference with the Python
QAPI generated files - another series).

I'll wait if we get to a consensus about what exactly is virtiofsd,
then revisit this series.

Thanks!

Phil.


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
  2021-04-28 18:00       ` [Virtio-fs] " Philippe Mathieu-Daudé
@ 2021-04-29  7:31         ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-29  7:31 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, Paolo Bonzini
  Cc: Thomas Huth, Daniel P . Berrange, Connor Kuehl,
	Dr. David Alan Gilbert, virtio-fs, Stefan Hajnoczi,
	Mahmoud Mandour, Alex Bennée

On 4/28/21 8:00 PM, Philippe Mathieu-Daudé wrote:
> On 4/28/21 6:34 PM, Richard Henderson wrote:
>> On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>>>   seccomp = not_found
>>> -if not get_option('seccomp').auto() or have_system or have_tools
>>> +if not get_option('seccomp').auto() or have_system or have_tools or
>>> not get_option('virtiofsd').auto()
>>>     seccomp = dependency('libseccomp', version: '>=2.3.0',
>>>                          required: get_option('seccomp'),
>>>                          method: 'pkg-config', kwargs: static_kwargs)
>>
>> This construct is wrong, both before and after, as I read it.
>>
>> not get_option(foo).auto() is true for both enabled and disabled.  If
>> disabled, why are we examining the dependency?  If auto, if we have all
>> of the dependencies we want to enable the feature -- if we don't probe
>> for the dependency, how can we enable it?
>>
>> This error seems to be offset by the OR have_* tests, for which the
>> logic also seems off.
>>
>> I think the test should have been
>>
>>   if (have_system or have_tools) and
> 
> Yes but virtiofsd is not a tool... It is a standalone binary.
> Maybe have_system is the culprit here:
> 
>   have_system = have_system or target.endswith('-softmmu')
> 
> We should somewhere add:
> 
>   have_system = have_system or something('virtiofsd')

So this hunk does fix the issue ...:

-- >8 --
--- a/meson.build
+++ b/meson.build
@@ -52,4 +52,5 @@
 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

---

> However I wonder if we aren't going to build many objects
> that are irrelevant for virtiofsd.

Based on top of
https://www.mail-archive.com/qemu-devel@nongnu.org/msg799069.html
to remove libsoftfloat, 216 objects are required to build virtiofsd:

[216/216] Linking target tools/virtiofsd/virtiofsd

This one-line fix seems good enough (to keep virtiofsd as a 'tool').



^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [Virtio-fs] [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled
@ 2021-04-29  7:31         ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 32+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-04-29  7:31 UTC (permalink / raw)
  To: Richard Henderson, qemu-devel, Paolo Bonzini
  Cc: Thomas Huth, Daniel P . Berrange, virtio-fs, Mahmoud Mandour,
	Alex Bennée

On 4/28/21 8:00 PM, Philippe Mathieu-Daudé wrote:
> On 4/28/21 6:34 PM, Richard Henderson wrote:
>> On 4/28/21 7:48 AM, Philippe Mathieu-Daudé wrote:
>>>   seccomp = not_found
>>> -if not get_option('seccomp').auto() or have_system or have_tools
>>> +if not get_option('seccomp').auto() or have_system or have_tools or
>>> not get_option('virtiofsd').auto()
>>>     seccomp = dependency('libseccomp', version: '>=2.3.0',
>>>                          required: get_option('seccomp'),
>>>                          method: 'pkg-config', kwargs: static_kwargs)
>>
>> This construct is wrong, both before and after, as I read it.
>>
>> not get_option(foo).auto() is true for both enabled and disabled.  If
>> disabled, why are we examining the dependency?  If auto, if we have all
>> of the dependencies we want to enable the feature -- if we don't probe
>> for the dependency, how can we enable it?
>>
>> This error seems to be offset by the OR have_* tests, for which the
>> logic also seems off.
>>
>> I think the test should have been
>>
>>   if (have_system or have_tools) and
> 
> Yes but virtiofsd is not a tool... It is a standalone binary.
> Maybe have_system is the culprit here:
> 
>   have_system = have_system or target.endswith('-softmmu')
> 
> We should somewhere add:
> 
>   have_system = have_system or something('virtiofsd')

So this hunk does fix the issue ...:

-- >8 --
--- a/meson.build
+++ b/meson.build
@@ -52,4 +52,5 @@
 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

---

> However I wonder if we aren't going to build many objects
> that are irrelevant for virtiofsd.

Based on top of
https://www.mail-archive.com/qemu-devel@nongnu.org/msg799069.html
to remove libsoftfloat, 216 objects are required to build virtiofsd:

[216/216] Linking target tools/virtiofsd/virtiofsd

This one-line fix seems good enough (to keep virtiofsd as a 'tool').


^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2021-04-29  7:34 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-28 14:48 [PATCH 0/2] virtiofsd: Meson build fixes Philippe Mathieu-Daudé
2021-04-28 14:48 ` [Virtio-fs] " Philippe Mathieu-Daudé
2021-04-28 14:48 ` [PATCH 1/2] meson: Check for seccomp/cap-ng libraries if virtiofsd is enabled Philippe Mathieu-Daudé
2021-04-28 14:48   ` [Virtio-fs] " Philippe Mathieu-Daudé
2021-04-28 15:06   ` Peter Maydell
2021-04-28 15:06     ` [Virtio-fs] " Peter Maydell
2021-04-28 15:34     ` Philippe Mathieu-Daudé
2021-04-28 15:34       ` [Virtio-fs] " Philippe Mathieu-Daudé
2021-04-28 16:34   ` Richard Henderson
2021-04-28 16:34     ` [Virtio-fs] " Richard Henderson
2021-04-28 18:00     ` Philippe Mathieu-Daudé
2021-04-28 18:00       ` [Virtio-fs] " Philippe Mathieu-Daudé
2021-04-28 18:36       ` Peter Maydell
2021-04-28 18:36         ` [Virtio-fs] " Peter Maydell
2021-04-29  7:31       ` Philippe Mathieu-Daudé
2021-04-29  7:31         ` [Virtio-fs] " Philippe Mathieu-Daudé
2021-04-28 14:48 ` [PATCH 2/2] util/meson: Build iov/hexdump/buffer_is_zero with virtiofsd Philippe Mathieu-Daudé
2021-04-28 14:48   ` [Virtio-fs] " Philippe Mathieu-Daudé
2021-04-28 14:52   ` Philippe Mathieu-Daudé
2021-04-28 14:52     ` [Virtio-fs] " Philippe Mathieu-Daudé
2021-04-28 16:38   ` Richard Henderson
2021-04-28 16:38     ` [Virtio-fs] " Richard Henderson
2021-04-28 17:56     ` Philippe Mathieu-Daudé
2021-04-28 17:56       ` [Virtio-fs] " Philippe Mathieu-Daudé
2021-04-28 19:24       ` Richard Henderson
2021-04-28 19:24         ` [Virtio-fs] " Richard Henderson
2021-04-28 19:34         ` Philippe Mathieu-Daudé
2021-04-28 19:34           ` [Virtio-fs] " Philippe Mathieu-Daudé
2021-04-28 15:15 ` [PATCH 0/2] virtiofsd: Meson build fixes Peter Maydell
2021-04-28 15:15   ` [Virtio-fs] " Peter Maydell
2021-04-28 16:52   ` Dr. David Alan Gilbert
2021-04-28 16:52     ` [Virtio-fs] " Dr. David Alan Gilbert

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.