[v4] cutils: Introduce bundle mechanism

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

Posted by Akihiko Odaki 2 years, 3 months ago

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;
 }
 
@@ -83,26 +94,6 @@ void qemu_add_data_dir(char *path)
     data_dir[data_dir_idx++] = path;
 }
 
-/*
- * Find a likely location for support files using the location of the binary.
- * When running from the build tree this will be "$bindir/pc-bios".
- * Otherwise, this is CONFIG_QEMU_DATADIR (possibly relocated).
- *
- * The caller must use g_free() to free the returned data when it is
- * no longer required.
- */
-static char *find_datadir(void)
-{
-    g_autofree char *dir = NULL;
-
-    dir = g_build_filename(qemu_get_exec_dir(), "pc-bios", NULL);
-    if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
-        return g_steal_pointer(&dir);
-    }
-
-    return get_relocated_path(CONFIG_QEMU_DATADIR);
-}
-
 void qemu_add_default_firmwarepath(void)
 {
     char **dirs;
@@ -114,9 +105,6 @@ void qemu_add_default_firmwarepath(void)
         qemu_add_data_dir(get_relocated_path(dirs[i]));
     }
     g_strfreev(dirs);
-
-    /* try to find datadir relative to the executable path */
-    qemu_add_data_dir(find_datadir());
 }
 
 void qemu_list_data_dirs(void)
@@ -125,4 +113,5 @@ void qemu_list_data_dirs(void)
     for (i = 0; i < data_dir_idx; i++) {
         printf("%s\n", data_dir[i]);
     }
+    list_bundle_candidates(CONFIG_QEMU_BUNDLE_DATADIR);
 }
diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 0ad4ba9e94d..2062b40d82b 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -174,21 +174,6 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
     target_name = strstr(**argv, "-target-");
     if (target_name) {        /* The binary name specifies the target */
         target_name += strlen("-target-");
-        /*
-         * With oss-fuzz, the executable is kept in the root of a directory (we
-         * cannot assume the path). All data (including bios binaries) must be
-         * in the same dir, or a subdir. Thus, we cannot place the pc-bios so
-         * that it would be in exec_dir/../pc-bios.
-         * As a workaround, oss-fuzz allows us to use argv[0] to get the
-         * location of the executable. Using this we add exec_dir/pc-bios to
-         * the datadirs.
-         */
-        bindir = qemu_get_exec_dir();
-        datadir = g_build_filename(bindir, "pc-bios", NULL);
-        if (g_file_test(datadir, G_FILE_TEST_IS_DIR)) {
-            qemu_add_data_dir(datadir);
-        } else {
-            g_free(datadir);
 	}
     } else if (*argc > 1) {  /* The target is specified as an argument */
         target_name = (*argv)[1];
diff --git a/tests/vm/fedora b/tests/vm/fedora
index 92b78d6e2c9..4ccd31bba61 100755
--- a/tests/vm/fedora
+++ b/tests/vm/fedora
@@ -79,7 +79,7 @@ class FedoraVM(basevm.BaseVM):
         self.exec_qemu_img("create", "-f", "qcow2", img_tmp, self.size)
         self.print_step("Booting installer")
         self.boot(img_tmp, extra_args = [
-            "-bios", "pc-bios/bios-256k.bin",
+            "-bios", "qemu-bundle/share/qemu/bios-256k.bin",
             "-machine", "graphics=off",
             "-device", "VGA",
             "-cdrom", iso
diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index 805db759d67..2095d8c5204 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -95,7 +95,7 @@ class FreeBSDVM(basevm.BaseVM):
 
         self.print_step("Booting installer")
         self.boot(img_tmp, extra_args = [
-            "-bios", "pc-bios/bios-256k.bin",
+            "-bios", "qemu-bundle/share/qemu/bios-256k.bin",
             "-machine", "graphics=off",
             "-device", "VGA",
             "-cdrom", iso
diff --git a/tests/vm/netbsd b/tests/vm/netbsd
index 45aa9a7fda7..d59cfedb83e 100755
--- a/tests/vm/netbsd
+++ b/tests/vm/netbsd
@@ -86,7 +86,7 @@ class NetBSDVM(basevm.BaseVM):
 
         self.print_step("Booting installer")
         self.boot(img_tmp, extra_args = [
-            "-bios", "pc-bios/bios-256k.bin",
+            "-bios", "qemu-bundle/share/qemu/bios-256k.bin",
             "-machine", "graphics=off",
             "-cdrom", iso
         ])
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index 13c82542140..036907c6243 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -82,7 +82,7 @@ class OpenBSDVM(basevm.BaseVM):
 
         self.print_step("Booting installer")
         self.boot(img_tmp, extra_args = [
-            "-bios", "pc-bios/bios-256k.bin",
+            "-bios", "qemu-bundle/share/qemu/bios-256k.bin",
             "-machine", "graphics=off",
             "-device", "VGA",
             "-cdrom", iso
-- 
2.32.1 (Apple Git-133)

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

Posted by Paolo Bonzini 2 years, 3 months ago

On 6/14/22 23:07, Akihiko Odaki wrote:
> )
> +
>   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)

This needs to be conditional on get_option('install_blobs').

Paolo

>   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')

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

Posted by Daniel P. Berrangé 2 years, 3 months ago

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 :|

[PATCH v4 1/4] cutils: Introduce bundle mechanism
[PATCH v4 2/4] datadir: Use bundle mechanism
[PATCH v4 3/4] ui/icons: Use bundle mechanism
[PATCH v4 4/4] net: Use bundle mechanism