* [PATCH v4 1/4] cutils: Introduce bundle mechanism
2022-06-14 21:07 [PATCH v4 0/4] cutils: Introduce bundle mechanism Akihiko Odaki
@ 2022-06-14 21:07 ` Akihiko Odaki
2022-06-15 8:19 ` Paolo Bonzini
2022-06-14 21:07 ` [PATCH v4 2/4] datadir: Use " Akihiko Odaki
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Akihiko Odaki @ 2022-06-14 21:07 UTC (permalink / raw)
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P . Berrangé, Akihiko Odaki
Developers often run QEMU without installing. The bundle mechanism
allows to look up files which should be present in installation even in
such a situation.
It is a general mechanism and can find any files located relative
to the installation tree. The build tree must have a new directory,
qemu-bundle, to represent what files the installation tree would
have for reference by the executables.
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
include/qemu/cutils.h | 19 +++++++++++++++++++
util/cutils.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/include/qemu/cutils.h b/include/qemu/cutils.h
index 40e10e19a7e..3b66026cd3c 100644
--- a/include/qemu/cutils.h
+++ b/include/qemu/cutils.h
@@ -213,6 +213,25 @@ const char *qemu_get_exec_dir(void);
*/
char *get_relocated_path(const char *dir);
+/**
+ * find_bundle:
+ * @path: Relative path
+ *
+ * Returns a path for the specified directory or file bundled in QEMU. It uses
+ * the directory of the running executable as the prefix first. See
+ * get_relocated_path() for the details. The next candidate is "qemu-bundle"
+ * directory in the directory of the running executable. "qemu-bundle"
+ * directory is typically present in the build tree.
+ *
+ * The returned string should be freed by the caller.
+ *
+ * Returns: a path that can access the bundle, or NULL if no matching bundle
+ * exists.
+ */
+char *find_bundle(const char *path);
+
+void list_bundle_candidates(const char *path);
+
static inline const char *yes_no(bool b)
{
return b ? "yes" : "no";
diff --git a/util/cutils.c b/util/cutils.c
index a58bcfd80e7..fe3bbb1c4eb 100644
--- a/util/cutils.c
+++ b/util/cutils.c
@@ -1086,3 +1086,36 @@ char *get_relocated_path(const char *dir)
}
return g_string_free(result, false);
}
+
+static const char * const bundle_formats[] = {
+ "%s" G_DIR_SEPARATOR_S ".." G_DIR_SEPARATOR_S "%s",
+ "%s" G_DIR_SEPARATOR_S "qemu-bundle" G_DIR_SEPARATOR_S "%s"
+};
+
+char *find_bundle(const char *path)
+{
+ const char *dir = qemu_get_exec_dir();
+ char *candidate;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(bundle_formats); i++) {
+ candidate = g_strdup_printf(bundle_formats[i], dir, path);
+ if (access(candidate, R_OK) == 0) {
+ return candidate;
+ }
+ g_free(candidate);
+ }
+
+ return NULL;
+}
+
+void list_bundle_candidates(const char *path)
+{
+ const char *dir = qemu_get_exec_dir();
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(bundle_formats); i++) {
+ printf(bundle_formats[i], dir, path);
+ putc('\n', stdout);
+ }
+}
--
2.32.1 (Apple Git-133)
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 1/4] cutils: Introduce bundle mechanism
2022-06-14 21:07 ` [PATCH v4 1/4] " Akihiko Odaki
@ 2022-06-15 8:19 ` Paolo Bonzini
2022-06-15 13:12 ` Akihiko Odaki
0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2022-06-15 8:19 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P . Berrangé
On 6/14/22 23:07, Akihiko Odaki wrote:
> diff --git a/util/cutils.c b/util/cutils.c
> index a58bcfd80e7..fe3bbb1c4eb 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -1086,3 +1086,36 @@ char *get_relocated_path(const char *dir)
> }
> return g_string_free(result, false);
> }
> +
> +static const char * const bundle_formats[] = {
> + "%s" G_DIR_SEPARATOR_S ".." G_DIR_SEPARATOR_S "%s",
> + "%s" G_DIR_SEPARATOR_S "qemu-bundle" G_DIR_SEPARATOR_S "%s"
> +};
Why do you need both?
Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 1/4] cutils: Introduce bundle mechanism
2022-06-15 8:19 ` Paolo Bonzini
@ 2022-06-15 13:12 ` Akihiko Odaki
0 siblings, 0 replies; 14+ messages in thread
From: Akihiko Odaki @ 2022-06-15 13:12 UTC (permalink / raw)
To: Paolo Bonzini, Daniel P . Berrangé
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann
On 2022/06/15 17:19, Paolo Bonzini wrote:
> On 6/14/22 23:07, Akihiko Odaki wrote:
>> diff --git a/util/cutils.c b/util/cutils.c
>> index a58bcfd80e7..fe3bbb1c4eb 100644
>> --- a/util/cutils.c
>> +++ b/util/cutils.c
>> @@ -1086,3 +1086,36 @@ char *get_relocated_path(const char *dir)
>> }
>> return g_string_free(result, false);
>> }
>> +
>> +static const char * const bundle_formats[] = {
>> + "%s" G_DIR_SEPARATOR_S ".." G_DIR_SEPARATOR_S "%s",
>> + "%s" G_DIR_SEPARATOR_S "qemu-bundle" G_DIR_SEPARATOR_S "%s"
>> +};
>
> Why do you need both?
>
> Paolo
The earlier one is used when QEMU is installed. The latter one is used
in the build tree.
Actually the order was problematic as Daniel pointed out. It is fixed in
the v5, which I have just sent out.
On 2022/06/15 17:16, Daniel P. Berrangé wrote:
> 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.
Regards,
Akihiko Odaki
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 2/4] datadir: Use bundle mechanism
2022-06-14 21:07 [PATCH v4 0/4] cutils: Introduce bundle mechanism Akihiko Odaki
2022-06-14 21:07 ` [PATCH v4 1/4] " Akihiko Odaki
@ 2022-06-14 21:07 ` Akihiko Odaki
2022-06-15 8:16 ` Daniel P. Berrangé
2022-06-15 8:21 ` Paolo Bonzini
2022-06-14 21:07 ` [PATCH v4 3/4] ui/icons: " Akihiko Odaki
` (3 subsequent siblings)
5 siblings, 2 replies; 14+ messages in thread
From: Akihiko Odaki @ 2022-06-14 21:07 UTC (permalink / raw)
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P . Berrangé, Akihiko Odaki
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)
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 2/4] datadir: Use bundle mechanism
2022-06-14 21:07 ` [PATCH v4 2/4] datadir: Use " Akihiko Odaki
@ 2022-06-15 8:16 ` Daniel P. Berrangé
2022-06-15 8:21 ` Paolo Bonzini
1 sibling, 0 replies; 14+ messages in thread
From: Daniel P. Berrangé @ 2022-06-15 8:16 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann
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 :|
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 2/4] datadir: Use bundle mechanism
2022-06-14 21:07 ` [PATCH v4 2/4] datadir: Use " Akihiko Odaki
2022-06-15 8:16 ` Daniel P. Berrangé
@ 2022-06-15 8:21 ` Paolo Bonzini
1 sibling, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2022-06-15 8:21 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P . Berrangé
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')
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v4 3/4] ui/icons: Use bundle mechanism
2022-06-14 21:07 [PATCH v4 0/4] cutils: Introduce bundle mechanism Akihiko Odaki
2022-06-14 21:07 ` [PATCH v4 1/4] " Akihiko Odaki
2022-06-14 21:07 ` [PATCH v4 2/4] datadir: Use " Akihiko Odaki
@ 2022-06-14 21:07 ` Akihiko Odaki
2022-06-14 21:07 ` [PATCH v4 4/4] net: " Akihiko Odaki
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Akihiko Odaki @ 2022-06-14 21:07 UTC (permalink / raw)
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P . Berrangé, Akihiko Odaki
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
meson.build | 2 +-
ui/cocoa.m | 29 ++++++++++++++++-------------
ui/gtk.c | 6 +++++-
ui/icons/meson.build | 36 ++++++++++++++++++++++++++++--------
ui/sdl2.c | 18 +++++++++++-------
5 files changed, 61 insertions(+), 30 deletions(-)
diff --git a/meson.build b/meson.build
index c573815813f..e7b385eaf34 100644
--- a/meson.build
+++ b/meson.build
@@ -1687,7 +1687,7 @@ 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'))
-config_host_data.set_quoted('CONFIG_QEMU_ICONDIR', get_option('prefix') / qemu_icondir)
+config_host_data.set_quoted('CONFIG_QEMU_BUNDLE_ICONDIR', qemu_icondir)
config_host_data.set_quoted('CONFIG_QEMU_LOCALEDIR', get_option('prefix') / get_option('localedir'))
config_host_data.set_quoted('CONFIG_QEMU_LOCALSTATEDIR', get_option('prefix') / get_option('localstatedir'))
config_host_data.set_quoted('CONFIG_QEMU_MODDIR', get_option('prefix') / qemu_moddir)
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 84c84e98fc5..bd8a3211d3b 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -1562,21 +1562,24 @@ - (BOOL)verifyQuit
- (IBAction) do_about_menu_item: (id) sender
{
NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
- char *icon_path_c = get_relocated_path(CONFIG_QEMU_ICONDIR "/hicolor/512x512/apps/qemu.png");
- NSString *icon_path = [NSString stringWithUTF8String:icon_path_c];
- g_free(icon_path_c);
- NSImage *icon = [[NSImage alloc] initWithContentsOfFile:icon_path];
+ char *icon_path_c = find_bundle(CONFIG_QEMU_BUNDLE_ICONDIR "/hicolor/512x512/apps/qemu.png");
NSString *version = @"QEMU emulator version " QEMU_FULL_VERSION;
NSString *copyright = @QEMU_COPYRIGHT;
- NSDictionary *options;
- if (icon) {
- options = @{
- NSAboutPanelOptionApplicationIcon : icon,
- NSAboutPanelOptionApplicationVersion : version,
- @"Copyright" : copyright,
- };
- [icon release];
- } else {
+ NSDictionary *options = nil;
+ if (icon_path_c) {
+ NSString *icon_path = [NSString stringWithUTF8String:icon_path_c];
+ g_free(icon_path_c);
+ NSImage *icon = [[NSImage alloc] initWithContentsOfFile:icon_path];
+ if (icon) {
+ options = @{
+ NSAboutPanelOptionApplicationIcon : icon,
+ NSAboutPanelOptionApplicationVersion : version,
+ @"Copyright" : copyright,
+ };
+ [icon release];
+ }
+ }
+ if (!options) {
options = @{
NSAboutPanelOptionApplicationVersion : version,
@"Copyright" : copyright,
diff --git a/ui/gtk.c b/ui/gtk.c
index 2a791dd2aa0..8f7afe795f4 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -2321,7 +2321,11 @@ static void gtk_display_init(DisplayState *ds, DisplayOptions *opts)
s->opts = opts;
theme = gtk_icon_theme_get_default();
- dir = get_relocated_path(CONFIG_QEMU_ICONDIR);
+ dir = find_bundle(CONFIG_QEMU_BUNDLE_ICONDIR);
+ if (dir) {
+ gtk_icon_theme_prepend_search_path(theme, dir);
+ g_free(dir);
+ }
gtk_icon_theme_prepend_search_path(theme, dir);
g_free(dir);
g_set_prgname("qemu");
diff --git a/ui/icons/meson.build b/ui/icons/meson.build
index 12c52080ebd..23292773074 100644
--- a/ui/icons/meson.build
+++ b/ui/icons/meson.build
@@ -1,13 +1,33 @@
+icons = [
+ {
+ 'source': 'qemu_32x32.bmp',
+ 'install': 'hicolor' / '32x32' / 'apps' / 'qemu.bmp',
+ },
+ {
+ 'source': 'qemu.svg',
+ 'install': 'hicolor' / 'scalable' / 'apps' / 'qemu.svg',
+ },
+]
+
foreach s: [16, 24, 32, 48, 64, 128, 256, 512]
s = '@0@x@0@'.format(s.to_string())
- install_data('qemu_@0@.png'.format(s),
- rename: 'qemu.png',
- install_dir: qemu_icondir / 'hicolor' / s / 'apps')
+ icons += {
+ 'source': 'qemu_@0@.png'.format(s),
+ 'install': 'hicolor' / s / 'apps' / 'qemu.png',
+ }
endforeach
-install_data('qemu_32x32.bmp',
- rename: 'qemu.bmp',
- install_dir: qemu_icondir / 'hicolor' / '32x32' / 'apps')
+foreach icon: icons
+ source = icon.get('source')
+ install = icon.get('install')
+
+ install_data(source,
+ rename: fs.name(install),
+ install_dir: qemu_icondir / fs.parent(install))
-install_data('qemu.svg',
- install_dir: qemu_icondir / 'hicolor' / 'scalable' / 'apps')
+ run_command('mkdir', '-p', qemu_bundledir / qemu_icondir / fs.parent(install),
+ check: true)
+
+ run_command('ln', '-sf', meson.current_source_dir() / source, qemu_bundledir / qemu_icondir / install,
+ check: true)
+endforeach
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 8cb77416af2..bbcb4762e1b 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -910,15 +910,19 @@ static void sdl2_display_init(DisplayState *ds, DisplayOptions *o)
}
#ifdef CONFIG_SDL_IMAGE
- dir = get_relocated_path(CONFIG_QEMU_ICONDIR "/hicolor/128x128/apps/qemu.png");
- icon = IMG_Load(dir);
+ dir = find_bundle(CONFIG_QEMU_BUNDLE_ICONDIR "/hicolor/128x128/apps/qemu.png");
+ if (dir) {
+ icon = IMG_Load(dir);
+ }
#else
/* Load a 32x32x4 image. White pixels are transparent. */
- dir = get_relocated_path(CONFIG_QEMU_ICONDIR "/hicolor/32x32/apps/qemu.bmp");
- icon = SDL_LoadBMP(dir);
- if (icon) {
- uint32_t colorkey = SDL_MapRGB(icon->format, 255, 255, 255);
- SDL_SetColorKey(icon, SDL_TRUE, colorkey);
+ dir = find_bundle(CONFIG_QEMU_BUNDLE_ICONDIR "/hicolor/32x32/apps/qemu.bmp");
+ if (dir) {
+ icon = SDL_LoadBMP(dir);
+ if (icon) {
+ uint32_t colorkey = SDL_MapRGB(icon->format, 255, 255, 255);
+ SDL_SetColorKey(icon, SDL_TRUE, colorkey);
+ }
}
#endif
g_free(dir);
--
2.32.1 (Apple Git-133)
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v4 4/4] net: Use bundle mechanism
2022-06-14 21:07 [PATCH v4 0/4] cutils: Introduce bundle mechanism Akihiko Odaki
` (2 preceding siblings ...)
2022-06-14 21:07 ` [PATCH v4 3/4] ui/icons: " Akihiko Odaki
@ 2022-06-14 21:07 ` Akihiko Odaki
2022-06-15 8:30 ` [PATCH v4 0/4] cutils: Introduce " Daniel P. Berrangé
2022-06-15 8:39 ` Paolo Bonzini
5 siblings, 0 replies; 14+ messages in thread
From: Akihiko Odaki @ 2022-06-14 21:07 UTC (permalink / raw)
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P . Berrangé, Akihiko Odaki
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
---
include/net/net.h | 2 +-
meson.build | 8 +++++++-
net/tap.c | 6 +++++-
qemu-options.hx | 4 ++--
4 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 523136c7acb..4a5ed27a4b7 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -228,7 +228,7 @@ NetClientState *net_hub_port_find(int hub_id);
#define DEFAULT_NETWORK_SCRIPT CONFIG_SYSCONFDIR "/qemu-ifup"
#define DEFAULT_NETWORK_DOWN_SCRIPT CONFIG_SYSCONFDIR "/qemu-ifdown"
-#define DEFAULT_BRIDGE_HELPER CONFIG_QEMU_HELPERDIR "/qemu-bridge-helper"
+#define DEFAULT_BUNDLE_BRIDGE_HELPER CONFIG_QEMU_BUNDLE_HELPERDIR "/qemu-bridge-helper"
#define DEFAULT_BRIDGE_INTERFACE "br0"
void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
diff --git a/meson.build b/meson.build
index e7b385eaf34..72d006f228e 100644
--- a/meson.build
+++ b/meson.build
@@ -1686,7 +1686,7 @@ config_host_data.set_quoted('CONFIG_QEMU_CONFDIR', get_option('prefix') / qemu_c
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'))
+config_host_data.set_quoted('CONFIG_QEMU_BUNDLE_HELPERDIR', get_option('libexecdir'))
config_host_data.set_quoted('CONFIG_QEMU_BUNDLE_ICONDIR', qemu_icondir)
config_host_data.set_quoted('CONFIG_QEMU_LOCALEDIR', get_option('prefix') / get_option('localedir'))
config_host_data.set_quoted('CONFIG_QEMU_LOCALSTATEDIR', get_option('prefix') / get_option('localstatedir'))
@@ -3573,6 +3573,12 @@ if have_tools
dependencies: [authz, crypto, io, qom, qemuutil,
libcap_ng, mpathpersist],
install: true)
+
+ run_command('mkdir', '-p', qemu_bundledir / get_option('libexecdir'),
+ check: true)
+
+ run_command('ln', '-sf', '../../qemu-bridge-helper', qemu_bundledir / get_option('libexecdir'),
+ check: true)
endif
if have_ivshmem
diff --git a/net/tap.c b/net/tap.c
index b3ddfd4a74b..ea013ca3873 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -507,7 +507,11 @@ static int net_bridge_run_helper(const char *helper, const char *bridge,
sigprocmask(SIG_BLOCK, &mask, &oldmask);
if (!helper) {
- helper = default_helper = get_relocated_path(DEFAULT_BRIDGE_HELPER);
+ helper = default_helper = find_bundle(DEFAULT_BUNDLE_BRIDGE_HELPER);
+ if (!helper) {
+ error_setg(errp, "bridge helper not found");
+ return -1;
+ }
}
if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
diff --git a/qemu-options.hx b/qemu-options.hx
index 377d22fbd82..1959db01061 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2665,7 +2665,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
" to configure it and 'dfile' (default=" DEFAULT_NETWORK_DOWN_SCRIPT ")\n"
" to deconfigure it\n"
" use '[down]script=no' to disable script execution\n"
- " use network helper 'helper' (default=" DEFAULT_BRIDGE_HELPER ") to\n"
+ " use network helper 'helper' (default=" DEFAULT_BUNDLE_BRIDGE_HELPER ") to\n"
" configure it\n"
" use 'fd=h' to connect to an already opened TAP interface\n"
" use 'fds=x:y:...:z' to connect to already opened multiqueue capable TAP interfaces\n"
@@ -2684,7 +2684,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
"-netdev bridge,id=str[,br=bridge][,helper=helper]\n"
" configure a host TAP network backend with ID 'str' that is\n"
" connected to a bridge (default=" DEFAULT_BRIDGE_INTERFACE ")\n"
- " using the program 'helper (default=" DEFAULT_BRIDGE_HELPER ")\n"
+ " using the program 'helper (default=" DEFAULT_BUNDLE_BRIDGE_HELPER ")\n"
#endif
#ifdef __linux__
"-netdev l2tpv3,id=str,src=srcaddr,dst=dstaddr[,srcport=srcport][,dstport=dstport]\n"
--
2.32.1 (Apple Git-133)
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v4 0/4] cutils: Introduce bundle mechanism
2022-06-14 21:07 [PATCH v4 0/4] cutils: Introduce bundle mechanism Akihiko Odaki
` (3 preceding siblings ...)
2022-06-14 21:07 ` [PATCH v4 4/4] net: " Akihiko Odaki
@ 2022-06-15 8:30 ` Daniel P. Berrangé
2022-06-15 11:02 ` Paolo Bonzini
2022-06-15 8:39 ` Paolo Bonzini
5 siblings, 1 reply; 14+ messages in thread
From: Daniel P. Berrangé @ 2022-06-15 8:30 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann
On Wed, Jun 15, 2022 at 06:07:42AM +0900, Akihiko Odaki wrote:
> Developers often run QEMU without installing. The bundle mechanism
> allows to look up files which should be present in installation even in
> such a situation.
>
> It is a general mechanism and can find any files located relative
> to the installation tree. The build tree must have a new directory,
> qemu-bundle, to represent what files the installation tree would
> have for reference by the executables.
I don't think this is an attractive approach to the problem,
because it results in us adding a bunch of meson rules to
simulate 'make install' within the build dir. This is undesirable
clutter IMHO, and can be solved more simply by just modifying the
qemu_find_file() method.
The core problem is the impl of qemu_find_file is taking the wrong
approach, in several ways, but mostly because of its use of a single
'data_dirs' array for all types of file. This is bad because it
has the assumption that build dir and install dir layouts match,
and second because when we add extra firmware data dirs, we don't
want this used for non-firmware files.
We need to separate out the handling of different types of resources
for this to work correctly.
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 :|
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 0/4] cutils: Introduce bundle mechanism
2022-06-15 8:30 ` [PATCH v4 0/4] cutils: Introduce " Daniel P. Berrangé
@ 2022-06-15 11:02 ` Paolo Bonzini
2022-06-15 11:27 ` Daniel P. Berrangé
0 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2022-06-15 11:02 UTC (permalink / raw)
To: Daniel P. Berrangé, Akihiko Odaki
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann
On 6/15/22 10:30, Daniel P. Berrangé wrote:
> I don't think this is an attractive approach to the problem,
> because it results in us adding a bunch of meson rules to
> simulate 'make install' within the build dir. This is undesirable
> clutter IMHO, and can be solved more simply by just modifying the
> qemu_find_file() method.
>
> The core problem is the impl of qemu_find_file is taking the wrong
> approach, in several ways, but mostly because of its use of a single
> 'data_dirs' array for all types of file. This is bad because it
> has the assumption that build dir and install dir layouts match,
> and second because when we add extra firmware data dirs, we don't
> want this used for non-firmware files.
>
> We need to separate out the handling of different types of resources
> for this to work correctly.
In some sense this is what Akihiko did - instead of separating them in
qemu_find_file(), the "pre-install" layout separates them in the
filesystem. While I had remarks on the implementation I think it's a
sensible approach.
The pre-install directory could even be created as a custom_target,
using the JSON files from Meson introspection.
Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 0/4] cutils: Introduce bundle mechanism
2022-06-15 11:02 ` Paolo Bonzini
@ 2022-06-15 11:27 ` Daniel P. Berrangé
0 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrangé @ 2022-06-15 11:27 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Akihiko Odaki, Peter Maydell, Jason Wang, qemu-devel,
Programmingkid, Philippe Mathieu-Daudé, Gerd Hoffmann
On Wed, Jun 15, 2022 at 01:02:08PM +0200, Paolo Bonzini wrote:
> On 6/15/22 10:30, Daniel P. Berrangé wrote:
> > I don't think this is an attractive approach to the problem,
> > because it results in us adding a bunch of meson rules to
> > simulate 'make install' within the build dir. This is undesirable
> > clutter IMHO, and can be solved more simply by just modifying the
> > qemu_find_file() method.
> >
> > The core problem is the impl of qemu_find_file is taking the wrong
> > approach, in several ways, but mostly because of its use of a single
> > 'data_dirs' array for all types of file. This is bad because it
> > has the assumption that build dir and install dir layouts match,
> > and second because when we add extra firmware data dirs, we don't
> > want this used for non-firmware files.
> >
> > We need to separate out the handling of different types of resources
> > for this to work correctly.
>
> In some sense this is what Akihiko did - instead of separating them in
> qemu_find_file(), the "pre-install" layout separates them in the filesystem.
> While I had remarks on the implementation I think it's a sensible approach.
>
> The pre-install directory could even be created as a custom_target, using
> the JSON files from Meson introspection.
Doing that is more complicated than just refactoring qemu_find_file,
such that its search locations can be tailored per file type, just
by setting a couple variables in the code IMHO.
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 :|
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4 0/4] cutils: Introduce bundle mechanism
2022-06-14 21:07 [PATCH v4 0/4] cutils: Introduce bundle mechanism Akihiko Odaki
` (4 preceding siblings ...)
2022-06-15 8:30 ` [PATCH v4 0/4] cutils: Introduce " Daniel P. Berrangé
@ 2022-06-15 8:39 ` Paolo Bonzini
2022-06-15 10:53 ` Daniel P. Berrangé
5 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2022-06-15 8:39 UTC (permalink / raw)
To: Akihiko Odaki
Cc: Peter Maydell, Jason Wang, qemu-devel, Programmingkid,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P . Berrangé
On 6/14/22 23:07, Akihiko Odaki wrote:
> Developers often run QEMU without installing. The bundle mechanism
> allows to look up files which should be present in installation even in
> such a situation.
>
> It is a general mechanism and can find any files located relative
> to the installation tree. The build tree must have a new directory,
> qemu-bundle, to represent what files the installation tree would
> have for reference by the executables.
>
> v4:
> * Add Daniel P. Berrangé to CC. Hopefully this helps merging his patch:
> https://mail.gnu.org/archive/html/qemu-devel/2022-06/msg02276.html
> * Rebased to the latest QEMU.
>
> v3:
> * Note that the bundle mechanism is for any files located relative to the
> installation tree including but not limited to datadir. (Peter Maydell)
> * Fix "bridge" typo (Philippe Mathieu-Daudé)
>
> v2: Rebased to the latest QEMU.
I like the idea, but I have a couple issues with the implementation:
- at the meson level, there is some repetition of mkdir and ln
run_commands. Perhaps you could just fill in a dictionary, and then do
something like
created_paths = {}
foreach source, dest: var
path = fs.parent(qemu_bundledir / dest)
created_paths += {path: true}
endforeach
run_command('mkdir', '-p', created_paths.keys())
foreach source, dest: var
run_command('ln', '-sf', meson.project_source_root() / source,
qemu_bundledir / dest)
endforeach
at the end of the toplevel meson.build.
- at the code level, it seems to me that this could reuse a lot of the
logic of get_relocated_path(). In particular, I would include $prefix
in the qemu_bundledir, so that the files in the bundle directory would
look like qemu-bundle/usr/share/qemu/bios.bin: just like an install that
uses DESTDIR. Then, if an uninstalled QEMU somehow returns
$exec_path/qemu-bundle/$prefix/$bindir for qemu_get_exec_dir() instead
of $exec_path, then get_relocated_path() will automatically return the
correct paths from qemu-bundle/.
Thanks,
Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v4 0/4] cutils: Introduce bundle mechanism
2022-06-15 8:39 ` Paolo Bonzini
@ 2022-06-15 10:53 ` Daniel P. Berrangé
0 siblings, 0 replies; 14+ messages in thread
From: Daniel P. Berrangé @ 2022-06-15 10:53 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Akihiko Odaki, Peter Maydell, Jason Wang, qemu-devel,
Programmingkid, Philippe Mathieu-Daudé, Gerd Hoffmann
On Wed, Jun 15, 2022 at 10:39:29AM +0200, Paolo Bonzini wrote:
> On 6/14/22 23:07, Akihiko Odaki wrote:
> > Developers often run QEMU without installing. The bundle mechanism
> > allows to look up files which should be present in installation even in
> > such a situation.
> >
> > It is a general mechanism and can find any files located relative
> > to the installation tree. The build tree must have a new directory,
> > qemu-bundle, to represent what files the installation tree would
> > have for reference by the executables.
> >
> > v4:
> > * Add Daniel P. Berrangé to CC. Hopefully this helps merging his patch:
> > https://mail.gnu.org/archive/html/qemu-devel/2022-06/msg02276.html
> > * Rebased to the latest QEMU.
> >
> > v3:
> > * Note that the bundle mechanism is for any files located relative to the
> > installation tree including but not limited to datadir. (Peter Maydell)
> > * Fix "bridge" typo (Philippe Mathieu-Daudé)
> >
> > v2: Rebased to the latest QEMU.
>
> I like the idea, but I have a couple issues with the implementation:
>
> - at the meson level, there is some repetition of mkdir and ln run_commands.
> Perhaps you could just fill in a dictionary, and then do something like
>
> created_paths = {}
> foreach source, dest: var
> path = fs.parent(qemu_bundledir / dest)
> created_paths += {path: true}
> endforeach
> run_command('mkdir', '-p', created_paths.keys())
> foreach source, dest: var
> run_command('ln', '-sf', meson.project_source_root() / source,
> qemu_bundledir / dest)
> endforeach
Per my other reply, IMHO, all the meson changes are redundant.
I've just sent a series that illustrates how we can improve the
qemu_find_file method so it correctly copes with install dir
vs build dir being different layouts, and be extensible to
any types of file (bios, keymaps, icons, helper exes, and
more).
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 :|
^ permalink raw reply [flat|nested] 14+ messages in thread