| Message ID | 20201007092913.1524199-1-misono.tomohiro@jp.fujitsu.com |
|---|---|
| State | New |
| Headers | show |
| Series | configure: add option for virtiofsd | expand |
On 07/10/20 11:29, Misono Tomohiro wrote: > Currently it is unknown whether virtiofsd will be built at > configuration time. It will be automatically built when dependency > is met. Also, required libraries are not clear. > > To make this clear, add configure option --{enable,disable}-virtiofsd. > The default is the same as current (enabled if available) like many > other options. When --enable-virtiofsd is given and dependency is not > met, we get: > > ERROR: virtiofsd requires libcap-ng devel, seccomp devel, vhost user and tools support > > In addition, configuration summary now includes virtiofsd entry: > > build virtiofs daemon: YES/NO > > Sidenote: this patch defines CONFIG_VIRTIOFSD for config-host.mak > to avoid duplicate dependency check in tools/meson.build. > > Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> Hi Misono, can you please handle the option via meson_options.txt? That is, just pass the value (auto/enabled/disabled) through from configure to meson, and handle the default in tools/meson.build. The logic will be something like this: have_virtiofsd = (targetos == 'linux' and 'CONFIG_SECCOMP' in config_host and 'CONFIG_LIBCAP_NG' in config_host) if get_option('virtiofsd').enabled() if not have_virtiofsd if targetos != 'linux' error('virtiofsd requires Linux') else error('virtiofsd requires libcap-ng-devel and seccomp-devel') endif endif elif get_option('virtiofsd').disabled() or not have_tools or \ not 'CONFIG_VHOST_USER' in config_host have_virtiofsd = false endif if have_virtiofsd subdir('virtiofsd') endif This is because, when adding the option, there are some conditions that should disable virtiofsd by default but can be overridden with --enable-virtiofsd. More information on how to create a new Meson option can be found in docs/devel/build-system.rst. Thanks, Paolo > -have_virtiofsd = (have_system and > - have_tools and > - 'CONFIG_LINUX' in config_host and > - 'CONFIG_SECCOMP' in config_host and > - 'CONFIG_LIBCAP_NG' in config_host and > - 'CONFIG_VHOST_USER' in config_host) > - > -if have_virtiofsd > +if 'CONFIG_VIRTIOFSD' in config_host > subdir('virtiofsd') > endif >
> On 07/10/20 11:29, Misono Tomohiro wrote: > > Currently it is unknown whether virtiofsd will be built at > > configuration time. It will be automatically built when dependency is > > met. Also, required libraries are not clear. > > > > To make this clear, add configure option --{enable,disable}-virtiofsd. > > The default is the same as current (enabled if available) like many > > other options. When --enable-virtiofsd is given and dependency is not > > met, we get: > > > > ERROR: virtiofsd requires libcap-ng devel, seccomp devel, vhost user > > and tools support > > > > In addition, configuration summary now includes virtiofsd entry: > > > > build virtiofs daemon: YES/NO > > > > Sidenote: this patch defines CONFIG_VIRTIOFSD for config-host.mak to > > avoid duplicate dependency check in tools/meson.build. > > > > Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> > > Hi Misono, > > can you please handle the option via meson_options.txt? That is, just pass the value (auto/enabled/disabled) through > from configure to meson, and handle the default in tools/meson.build. The logic will be something like this: > > have_virtiofsd = (targetos == 'linux' and > 'CONFIG_SECCOMP' in config_host and > 'CONFIG_LIBCAP_NG' in config_host) > > if get_option('virtiofsd').enabled() > if not have_virtiofsd > if targetos != 'linux' > error('virtiofsd requires Linux') > else > error('virtiofsd requires libcap-ng-devel and seccomp-devel') > endif > endif > elif get_option('virtiofsd').disabled() or not have_tools or \ > not 'CONFIG_VHOST_USER' in config_host > have_virtiofsd = false > endif > > if have_virtiofsd > subdir('virtiofsd') > endif > > This is because, when adding the option, there are some conditions that should disable virtiofsd by default but can be > overridden with --enable-virtiofsd. > > More information on how to create a new Meson option can be found in docs/devel/build-system.rst. Hi Paolo Thanks a lot for the clear explanation. I will update the patch to follow the meson style. I realized virtiofsd actually needs tools (i.e. "--disable-tools --enable-virtiofsd" does not work with above meson.build) since virtiofsd requires libvhost_user which will be built ony when tools are built. So, I will keep the current dependency check (except 'have_system'). BTW, while testing the updated patch, I noticed current master branch (as of 10/08) fails to execute virtiofsd. backtrace from coredump shows: #0 get_relocated_path (dir=0x560f4d2f2ef0 "/usr/local/var/run/virtiofsd") at ../util/cutils.c:924 #1 0x0000560f4baab6da in qemu_get_local_state_pathname (relative_pathname=relative_pathname@entry=0x560f4bac6cf1 "run/virtiofsd") at ../util/oslib-posix.c:345 #2 0x0000560f4baa1b09 in fv_socket_lock (se=0x560f4d2f2f20) at ../tools/virtiofsd/fuse_virtio.c:865 #3 fv_create_listen_socket (se=0x560f4d2f2f20) at ../tools/virtiofsd/fuse_virtio.c:906 #4 virtio_session_mount (se=0x560f4d2f2f20) at ../tools/virtiofsd/fuse_virtio.c:968 #5 0x0000560f4ba99725 in main (argc=<optimized out>, argv=<optimized out>) at ../tools/virtiofsd/passthrough_ll.c:2947 So, this is related to: https://github.com/qemu/qemu/commit/f4f5ed2cbde65acaa1bd88d00cc23fa8bf6b5ed9#diff-ae9b732998587b609c0854bae40b2fe6R924 Adding "qemu_init_exec_dir(argv[0]);" in virtiofs's main() seems solve the problem, but is it correct fix? Regards, Misono > > Thanks, > > Paolo > > > -have_virtiofsd = (have_system and > > - have_tools and > > - 'CONFIG_LINUX' in config_host and > > - 'CONFIG_SECCOMP' in config_host and > > - 'CONFIG_LIBCAP_NG' in config_host and > > - 'CONFIG_VHOST_USER' in config_host) > > - > > -if have_virtiofsd > > +if 'CONFIG_VIRTIOFSD' in config_host > > subdir('virtiofsd') > > endif > >
* misono.tomohiro@fujitsu.com (misono.tomohiro@fujitsu.com) wrote: > > On 07/10/20 11:29, Misono Tomohiro wrote: > > > Currently it is unknown whether virtiofsd will be built at > > > configuration time. It will be automatically built when dependency is > > > met. Also, required libraries are not clear. > > > > > > To make this clear, add configure option --{enable,disable}-virtiofsd. > > > The default is the same as current (enabled if available) like many > > > other options. When --enable-virtiofsd is given and dependency is not > > > met, we get: > > > > > > ERROR: virtiofsd requires libcap-ng devel, seccomp devel, vhost user > > > and tools support > > > > > > In addition, configuration summary now includes virtiofsd entry: > > > > > > build virtiofs daemon: YES/NO > > > > > > Sidenote: this patch defines CONFIG_VIRTIOFSD for config-host.mak to > > > avoid duplicate dependency check in tools/meson.build. > > > > > > Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> > > > > Hi Misono, > > > > can you please handle the option via meson_options.txt? That is, just pass the value (auto/enabled/disabled) through > > from configure to meson, and handle the default in tools/meson.build. The logic will be something like this: > > > > have_virtiofsd = (targetos == 'linux' and > > 'CONFIG_SECCOMP' in config_host and > > 'CONFIG_LIBCAP_NG' in config_host) > > > > if get_option('virtiofsd').enabled() > > if not have_virtiofsd > > if targetos != 'linux' > > error('virtiofsd requires Linux') > > else > > error('virtiofsd requires libcap-ng-devel and seccomp-devel') > > endif > > endif > > elif get_option('virtiofsd').disabled() or not have_tools or \ > > not 'CONFIG_VHOST_USER' in config_host > > have_virtiofsd = false > > endif > > > > if have_virtiofsd > > subdir('virtiofsd') > > endif > > > > This is because, when adding the option, there are some conditions that should disable virtiofsd by default but can be > > overridden with --enable-virtiofsd. > > > > More information on how to create a new Meson option can be found in docs/devel/build-system.rst. > > Hi Paolo > > Thanks a lot for the clear explanation. I will update the patch to follow the meson style. > I realized virtiofsd actually needs tools (i.e. "--disable-tools --enable-virtiofsd" > does not work with above meson.build) since virtiofsd requires libvhost_user which will > be built ony when tools are built. So, I will keep the current dependency check (except 'have_system'). > > BTW, while testing the updated patch, I noticed current master branch (as of 10/08) fails to execute virtiofsd. > backtrace from coredump shows: > #0 get_relocated_path (dir=0x560f4d2f2ef0 "/usr/local/var/run/virtiofsd") at ../util/cutils.c:924 > #1 0x0000560f4baab6da in qemu_get_local_state_pathname (relative_pathname=relative_pathname@entry=0x560f4bac6cf1 "run/virtiofsd") > at ../util/oslib-posix.c:345 > #2 0x0000560f4baa1b09 in fv_socket_lock (se=0x560f4d2f2f20) at ../tools/virtiofsd/fuse_virtio.c:865 > #3 fv_create_listen_socket (se=0x560f4d2f2f20) at ../tools/virtiofsd/fuse_virtio.c:906 > #4 virtio_session_mount (se=0x560f4d2f2f20) at ../tools/virtiofsd/fuse_virtio.c:968 > #5 0x0000560f4ba99725 in main (argc=<optimized out>, argv=<optimized out>) at ../tools/virtiofsd/passthrough_ll.c:2947 > > So, this is related to: https://github.com/qemu/qemu/commit/f4f5ed2cbde65acaa1bd88d00cc23fa8bf6b5ed9#diff-ae9b732998587b609c0854bae40b2fe6R924 > > Adding "qemu_init_exec_dir(argv[0]);" in virtiofs's main() seems solve the problem, > but is it correct fix? Yes, I've already posted the fix for that, https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg01852.html Dave > Regards, > Misono > > > > > Thanks, > > > > Paolo > > > > > -have_virtiofsd = (have_system and > > > - have_tools and > > > - 'CONFIG_LINUX' in config_host and > > > - 'CONFIG_SECCOMP' in config_host and > > > - 'CONFIG_LIBCAP_NG' in config_host and > > > - 'CONFIG_VHOST_USER' in config_host) > > > - > > > -if have_virtiofsd > > > +if 'CONFIG_VIRTIOFSD' in config_host > > > subdir('virtiofsd') > > > endif > > > > > > _______________________________________________ > Virtio-fs mailing list > Virtio-fs@redhat.com > https://www.redhat.com/mailman/listinfo/virtio-fs -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
... > > BTW, while testing the updated patch, I noticed current master branch (as of 10/08) fails to execute virtiofsd. > > backtrace from coredump shows: > > #0 get_relocated_path (dir=0x560f4d2f2ef0 > > "/usr/local/var/run/virtiofsd") at ../util/cutils.c:924 > > #1 0x0000560f4baab6da in qemu_get_local_state_pathname > (relative_pathname=relative_pathname@entry=0x560f4bac6cf1 "run/virtiofsd") > > at ../util/oslib-posix.c:345 > > #2 0x0000560f4baa1b09 in fv_socket_lock (se=0x560f4d2f2f20) at > > ../tools/virtiofsd/fuse_virtio.c:865 > > #3 fv_create_listen_socket (se=0x560f4d2f2f20) at > > ../tools/virtiofsd/fuse_virtio.c:906 > > #4 virtio_session_mount (se=0x560f4d2f2f20) at > > ../tools/virtiofsd/fuse_virtio.c:968 > > #5 0x0000560f4ba99725 in main (argc=<optimized out>, argv=<optimized > > out>) at ../tools/virtiofsd/passthrough_ll.c:2947 > > > > So, this is related to: > > https://github.com/qemu/qemu/commit/f4f5ed2cbde65acaa1bd88d00cc23fa8bf > > 6b5ed9#diff-ae9b732998587b609c0854bae40b2fe6R924 > > > > Adding "qemu_init_exec_dir(argv[0]);" in virtiofs's main() seems > > solve the problem, but is it correct fix? > > Yes, I've already posted the fix for that, > https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg01852.html > Thanks, then test is ok and I will send v2. Regards, Misono > Dave > > > Regards, > > Misono > > > > > > > > Thanks, > > > > > > Paolo > > > > > > > -have_virtiofsd = (have_system and > > > > - have_tools and > > > > - 'CONFIG_LINUX' in config_host and > > > > - 'CONFIG_SECCOMP' in config_host and > > > > - 'CONFIG_LIBCAP_NG' in config_host and > > > > - 'CONFIG_VHOST_USER' in config_host) > > > > - > > > > -if have_virtiofsd > > > > +if 'CONFIG_VIRTIOFSD' in config_host > > > > subdir('virtiofsd') > > > > endif > > > > > > > > > > _______________________________________________ > > Virtio-fs mailing list > > Virtio-fs@redhat.com > > https://www.redhat.com/mailman/listinfo/virtio-fs > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 08/10/20 11:17, misono.tomohiro@fujitsu.com wrote: > Hi Paolo > > Thanks a lot for the clear explanation. I will update the patch to follow the meson style. > I realized virtiofsd actually needs tools (i.e. "--disable-tools --enable-virtiofsd" > does not work with above meson.build) since virtiofsd requires libvhost_user which will > be built ony when tools are built. So, I will keep the current dependency check (except 'have_system'). I'm thinking the behavior for --disable-tools --enable-virtiofsd would be confusing. Therefore, another possibility is not introducing --enable-virtiofsd. Instead, you can reuse --enable-vhost-user-fs: - --enable-vhost-user-fs will fail if tools are enabled and cap-ng or seccomp are unavailable - --enable-vhost-user-fs --disable-tools will not look for cap-ng or seccomp, because then the flag only controls inclusion of vhost-user-fs in the emulators - if "--enable-vhost-user-fs" is not specified and tools are enabled, vhost-user-fs will only be included in the emulators if cap-ng and seccomp are available - if "--enable-vhost-user-fs" is not specified, tools are enabled and cap-ng/seccomp are unavilable, vhost-user-fs will not be included in the emulators either - if "--enable-vhost-user-fs" is not specified but tools are not enabled, configure will not check if cap-ng or seccomp. In this case reusing most of your previous patch, and not moving everything to meson, is totally okay. I don't want you to impose more transition work. Paolo
diff --git a/configure b/configure index ecc8e90e8b..9d76e73014 100755 --- a/configure +++ b/configure @@ -403,6 +403,7 @@ netmap="no" sdl="auto" sdl_image="auto" virtfs="" +virtiofsd="" mpath="" vnc="enabled" sparse="no" @@ -1117,6 +1118,10 @@ for opt do ;; --enable-virtfs) virtfs="yes" ;; + --disable-virtiofsd) virtiofsd="no" + ;; + --enable-virtiofsd) virtiofsd="yes" + ;; --disable-mpath) mpath="no" ;; --enable-mpath) mpath="yes" @@ -1873,6 +1878,7 @@ disabled with --disable-FEATURE, default is enabled if available: vnc-png PNG compression for VNC server cocoa Cocoa UI (Mac OS X only) virtfs VirtFS + virtiofsd build virtiofs daemon (virtiofsd) mpath Multipath persistent reservation passthrough xen xen backend driver support xen-pci-passthrough PCI passthrough support for Xen @@ -6344,6 +6350,15 @@ if test "$softmmu" = yes ; then fi virtfs=no fi + if test "$virtiofsd" != no && test "$cap_ng" = yes && test "$seccomp" = yes \ + && test "$vhost_user" = yes && test "$want_tools" = yes; then + virtiofsd=yes + else + if test "$virtiofsd" = yes; then + error_exit "virtiofsd requires libcap-ng devel, seccomp devel, vhost user and tools support" + fi + virtiofsd=no + fi if test "$mpath" != no && test "$mpathpersist" = yes ; then mpath=yes else @@ -6357,6 +6372,10 @@ if test "$softmmu" = yes ; then error_exit "VirtFS is supported only on Linux" fi virtfs=no + if test "$virtiofsd" = yes; then + error_exit "virtiofsd is supported only on Linux" + fi + virtiofsd=no if test "$mpath" = yes; then error_exit "Multipath is supported only on Linux" fi @@ -6901,6 +6920,9 @@ fi if test "$virtfs" = "yes" ; then echo "CONFIG_VIRTFS=y" >> $config_host_mak fi +if test "$virtiofsd" = "yes" ; then + echo "CONFIG_VIRTIOFSD=y" >> $config_host_mak +fi if test "$mpath" = "yes" ; then echo "CONFIG_MPATH=y" >> $config_host_mak if test "$mpathpersist_new_api" = "yes"; then diff --git a/docs/meson.build b/docs/meson.build index 0340d489ac..6b9b277ef7 100644 --- a/docs/meson.build +++ b/docs/meson.build @@ -15,7 +15,7 @@ if build_docs 'qemu-nbd.8': (have_tools ? 'man8' : ''), 'qemu-trace-stap.1': (config_host.has_key('CONFIG_TRACE_SYSTEMTAP') ? 'man1' : ''), 'virtfs-proxy-helper.1': (have_virtfs_proxy_helper ? 'man1' : ''), - 'virtiofsd.1': (have_virtiofsd ? 'man1' : ''), + 'virtiofsd.1': (config_host.has_key('CONFIG_VIRTIOFSD') ? 'man1' : ''), }, 'system': { 'qemu.1': 'man1', diff --git a/meson.build b/meson.build index 5b586afc38..a4ea961272 100644 --- a/meson.build +++ b/meson.build @@ -1336,6 +1336,7 @@ summary_info += {'Audio drivers': config_host['CONFIG_AUDIO_DRIVERS']} summary_info += {'Block whitelist (rw)': config_host['CONFIG_BDRV_RW_WHITELIST']} summary_info += {'Block whitelist (ro)': config_host['CONFIG_BDRV_RO_WHITELIST']} summary_info += {'VirtFS support': config_host.has_key('CONFIG_VIRTFS')} +summary_info += {'build virtiofs daemon': config_host.has_key('CONFIG_VIRTIOFSD')} summary_info += {'Multipath support': config_host.has_key('CONFIG_MPATH')} summary_info += {'VNC support': vnc.found()} if vnc.found() diff --git a/tools/meson.build b/tools/meson.build index 513bd2ff4f..f1241982d6 100644 --- a/tools/meson.build +++ b/tools/meson.build @@ -1,10 +1,3 @@ -have_virtiofsd = (have_system and - have_tools and - 'CONFIG_LINUX' in config_host and - 'CONFIG_SECCOMP' in config_host and - 'CONFIG_LIBCAP_NG' in config_host and - 'CONFIG_VHOST_USER' in config_host) - -if have_virtiofsd +if 'CONFIG_VIRTIOFSD' in config_host subdir('virtiofsd') endif
Currently it is unknown whether virtiofsd will be built at configuration time. It will be automatically built when dependency is met. Also, required libraries are not clear. To make this clear, add configure option --{enable,disable}-virtiofsd. The default is the same as current (enabled if available) like many other options. When --enable-virtiofsd is given and dependency is not met, we get: ERROR: virtiofsd requires libcap-ng devel, seccomp devel, vhost user and tools support In addition, configuration summary now includes virtiofsd entry: build virtiofs daemon: YES/NO Sidenote: this patch defines CONFIG_VIRTIOFSD for config-host.mak to avoid duplicate dependency check in tools/meson.build. Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> --- configure | 22 ++++++++++++++++++++++ docs/meson.build | 2 +- meson.build | 1 + tools/meson.build | 9 +-------- 4 files changed, 25 insertions(+), 9 deletions(-)