Re: [PATCH v4 2/4] datadir: Use bundle mechanism

[Daniel P . Berrangé]

On Wed, Jun 15, 2022 at 06:07:44AM +0900, Akihiko Odaki wrote:
> softmmu/datadir.c had its own implementation to find files in the
> build tree, but now bundle mechanism provides the unified
> implementation which works for datadir and the other files.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
> ---
>  .travis.yml                 |  2 +-
>  meson.build                 |  3 ++-
>  pc-bios/keymaps/meson.build |  3 +++
>  pc-bios/meson.build         | 17 +++++++++--------
>  scripts/oss-fuzz/build.sh   |  2 +-
>  softmmu/datadir.c           | 35 ++++++++++++-----------------------
>  tests/qtest/fuzz/fuzz.c     | 15 ---------------
>  tests/vm/fedora             |  2 +-
>  tests/vm/freebsd            |  2 +-
>  tests/vm/netbsd             |  2 +-
>  tests/vm/openbsd            |  2 +-
>  11 files changed, 32 insertions(+), 53 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 9afc4a54b8f..9fee2167b95 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -223,7 +223,7 @@ jobs:
>          - BUILD_RC=0 && make -j${JOBS} || BUILD_RC=$?
>          - |
>            if [ "$BUILD_RC" -eq 0 ] ; then
> -              mv pc-bios/s390-ccw/*.img pc-bios/ ;
> +              mv pc-bios/s390-ccw/*.img qemu-bundle/share/qemu ;
>                ${TEST_CMD} ;
>            else
>                $(exit $BUILD_RC);
> diff --git a/meson.build b/meson.build
> index 0c2e11ff071..c573815813f 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -32,6 +32,7 @@ if get_option('qemu_suffix').startswith('/')
>    error('qemu_suffix cannot start with a /')
>  endif
>  
> +qemu_bundledir = meson.project_build_root() / 'qemu-bundle'
>  qemu_confdir = get_option('sysconfdir') / get_option('qemu_suffix')
>  qemu_datadir = get_option('datadir') / get_option('qemu_suffix')
>  qemu_docdir = get_option('docdir') / get_option('qemu_suffix')
> @@ -1682,7 +1683,7 @@ endif
>  config_host_data.set_quoted('CONFIG_BINDIR', get_option('prefix') / 
> get_option('bindir'))
>  config_host_data.set_quoted('CONFIG_PREFIX', get_option('prefix'))
>  config_host_data.set_quoted('CONFIG_QEMU_CONFDIR', get_option('prefix') / 
> qemu_confdir)
> -config_host_data.set_quoted('CONFIG_QEMU_DATADIR', get_option('prefix') / 
> qemu_datadir)
> +config_host_data.set_quoted('CONFIG_QEMU_BUNDLE_DATADIR', qemu_datadir)
>  config_host_data.set_quoted('CONFIG_QEMU_DESKTOPDIR', get_option('prefix') / 
> qemu_desktopdir)
>  config_host_data.set_quoted('CONFIG_QEMU_FIRMWAREPATH', get_option('prefix') 
> / get_option('qemu_firmwarepath'))
>  config_host_data.set_quoted('CONFIG_QEMU_HELPERDIR', get_option('prefix') / 
> get_option('libexecdir'))
> diff --git a/pc-bios/keymaps/meson.build b/pc-bios/keymaps/meson.build
> index 44247a12b54..b8bac138756 100644
> --- a/pc-bios/keymaps/meson.build
> +++ b/pc-bios/keymaps/meson.build
> @@ -67,3 +67,6 @@ if native_qemu_keymap.found()
>  endif
>  
>  install_data(['sl', 'sv'], install_dir: qemu_datadir / 'keymaps')
> +
> +run_command('ln', '-sf', '../../../pc-bios/keymaps', qemu_bundledir / 
> qemu_datadir,
> +            check: true)
> diff --git a/pc-bios/meson.build b/pc-bios/meson.build
> index 41ba1c0ec7b..d1ff75b0b13 100644
> --- a/pc-bios/meson.build
> +++ b/pc-bios/meson.build
> @@ -1,3 +1,5 @@
> +run_command('mkdir', '-p', qemu_bundledir / qemu_datadir, check: true)
> +
>  roms = []
>  if unpack_edk2_blobs
>    fds = [
> @@ -20,6 +22,9 @@ if unpack_edk2_blobs
>                    install: get_option('install_blobs'),
>                    install_dir: qemu_datadir,
>                    command: [ bzip2, '-dc', '@INPUT0@' ])
> +
> +    run_command('ln', '-sf', '../../../pc-bios' / f, qemu_bundledir / 
> qemu_datadir,
> +                check: true)
>    endforeach
>  endif
>  
> @@ -85,15 +90,11 @@ blobs = [
>    'vof-nvram.bin',
>  ]
>  
> -ln_s = [find_program('ln', required: true), '-sf']
> +install_data(blobs, install_dir: qemu_datadir)
> +
>  foreach f : blobs
> -  roms += custom_target(f,
> -                build_by_default: have_system,
> -                output: f,
> -                input: files('meson.build'),            # dummy input
> -                install: get_option('install_blobs'),
> -                install_dir: qemu_datadir,
> -                command: [ ln_s, meson.project_source_root() / 'pc-bios' / 
> f, '@OUTPUT@' ])
> +  run_command('ln', '-sf', meson.current_source_dir() / f, qemu_bundledir / 
> qemu_datadir,
> +              check: true)
>  endforeach
>  
>  subdir('descriptors')
> diff --git a/scripts/oss-fuzz/build.sh b/scripts/oss-fuzz/build.sh
> index 98b56e05210..cbf8b3080e9 100755
> --- a/scripts/oss-fuzz/build.sh
> +++ b/scripts/oss-fuzz/build.sh
> @@ -88,7 +88,7 @@ if [ "$GITLAB_CI" != "true" ]; then
>  fi
>  
>  # Copy over the datadir
> -cp  -r ../pc-bios/ "$DEST_DIR/pc-bios"
> +cp  -r ../pc-bios/ "$DEST_DIR/qemu-bundle/share/qemu"
>  
>  targets=$(./qemu-fuzz-i386 | awk '$1 ~ /\*/  {print $2}')
>  base_copy="$DEST_DIR/qemu-fuzz-i386-target-$(echo "$targets" | head -n 1)"
> diff --git a/softmmu/datadir.c b/softmmu/datadir.c
> index 160cac999a6..4dadf0e010c 100644
> --- a/softmmu/datadir.c
> +++ b/softmmu/datadir.c
> @@ -35,6 +35,7 @@ char *qemu_find_file(int type, const char *name)
>      int i;
>      const char *subdir;
>      char *buf;
> +    char *bundle;
>  
>      /* Try the name as a straight path first */
>      if (access(name, R_OK) == 0) {
> @@ -61,6 +62,16 @@ char *qemu_find_file(int type, const char *name)
>          }
>          g_free(buf);
>      }
> +
> +    bundle = g_strdup_printf("%s/%s%s",
> +                             CONFIG_QEMU_BUNDLE_DATADIR, subdir, name);
> +    buf = find_bundle(bundle);
> +    g_free(bundle);
> +    if (buf) {
> +        trace_load_file(name, buf);
> +        return buf;
> +    }
> +
>      return NULL;
>  }

This is flawed because it looks at the installed paths first, and
falls back to uninstalled paths afterwards. So if you're building
and running QEMU 7.1.0 from git, and have QEMU 5.0.0 installed,
your QEMU 7.1.0 will end up finding files from the 5.0.0 install.


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