* [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.