* [PATCH v3 1/4] softmmu: rewrite handling of qemu_find_file
2022-06-15 10:52 [PATCH v3 0/4] softmmu: make qemu_find_file more flexible wrt build dir layout Daniel P. Berrangé
@ 2022-06-15 10:52 ` Daniel P. Berrangé
2022-06-15 10:52 ` [PATCH v3 2/4] ui: move 'pc-bios/keymaps' to 'ui/keymaps' Daniel P. Berrangé
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2022-06-15 10:52 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, Bandan Das, Thomas Huth, Volker Rümelin,
Darren Kenny, Qiuhao Li, Jason Wang, Peter Maydell, Akihiko Odaki,
Stefan Hajnoczi, Laurent Vivier, Paolo Bonzini,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P. Berrangé
The qemu_find_file method has a couple of flaws in its current
implementation:
* The configure time 'qemu-firmware' search path is mistakenly
also used to find keymaps
* The configure time 'qemu-firmware' search path is mistakenly
relocated even when running from build dir resulting in
non-sensical paths that won't resolve
* When searching for files it has the assumption that the
in-build-tree layout will match the installed root layout
The latter problem has forced us to keep the keymap files under a
sub-dir of the pc-bios/ instead of ui/.
This all stems from the way qemu_find_file tries to use a single
list of data directory locations, appending a type specific
subdir.
This can be addressed by refactoring the logic as follows
For each type of file to be found identify
* Optional: any user specified dir (non-relocated)
* Path relative to build dir
* Path relative to install dir
* Optional: extra configure time install dirs (bios only, relocated)
* The default install directory (relocated)
We can then search through:
* User specified dir
* If running from build dir
- Path relative to build dir
else
- Extra configure time dirs
- Path relative to default install dir
This is now flexible enough to extend to find any type of file,
by plugging in different input values, regardless of what layout
might be used in build dir vs install dir.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/qemu/datadir.h | 5 +-
softmmu/datadir.c | 145 +++++++++++++++++++++++-----------------
softmmu/trace-events | 5 +-
softmmu/vl.c | 2 +-
tests/qtest/fuzz/fuzz.c | 2 +-
5 files changed, 92 insertions(+), 67 deletions(-)
diff --git a/include/qemu/datadir.h b/include/qemu/datadir.h
index 21f9097f58..a333cd9b0d 100644
--- a/include/qemu/datadir.h
+++ b/include/qemu/datadir.h
@@ -15,6 +15,9 @@
* configured at build time (DATADIR) or registered with the -L command
* line option.
*
+ * @name may be NULL to indicate the caller just wants the
+ * first search directory that is found.
+ *
* The caller must use g_free() to free the returned data when it is
* no longer required.
*
@@ -22,7 +25,7 @@
*/
char *qemu_find_file(int type, const char *name);
void qemu_add_default_firmwarepath(void);
-void qemu_add_data_dir(char *path);
+void qemu_set_user_data_dir(const char *path);
void qemu_list_data_dirs(void);
#endif
diff --git a/softmmu/datadir.c b/softmmu/datadir.c
index 160cac999a..7457717542 100644
--- a/softmmu/datadir.c
+++ b/softmmu/datadir.c
@@ -27,102 +27,121 @@
#include "qemu/cutils.h"
#include "trace.h"
-static const char *data_dir[16];
-static int data_dir_idx;
+/* User specified data directory */
+static char *user_data_dir;
+
+/* Extra build time defined search locations for firmware (NULL terminated) */
+static char **extra_firmware_dirs;
+
+/* Default built-in directories */
+static char *default_data_dir;
+
+/* Whether we're known to be executing from a build tree */
+static bool in_build_dir;
char *qemu_find_file(int type, const char *name)
{
- int i;
- const char *subdir;
- char *buf;
-
- /* Try the name as a straight path first */
- if (access(name, R_OK) == 0) {
- trace_load_file(name, name);
- return g_strdup(name);
- }
+ const char *user_install_dir = NULL;
+ char **extra_install_dirs = NULL;
+ const char *rel_build_dir;
+ const char *rel_install_dir;
+ const char *default_install_dir;
+ char *maybepath = NULL;
+ size_t i;
+ int ret;
switch (type) {
case QEMU_FILE_TYPE_BIOS:
- subdir = "";
+ user_install_dir = user_data_dir;
+ extra_install_dirs = extra_firmware_dirs;
+ rel_install_dir = "";
+ rel_build_dir = "pc-bios";
+ default_install_dir = default_data_dir;
break;
+
case QEMU_FILE_TYPE_KEYMAP:
- subdir = "keymaps/";
+ user_install_dir = user_data_dir;
+ rel_install_dir = "keymaps";
+ rel_build_dir = "pc-bios/keymaps";
+ default_install_dir = default_data_dir;
break;
+
default:
abort();
}
- for (i = 0; i < data_dir_idx; i++) {
- buf = g_strdup_printf("%s/%s%s", data_dir[i], subdir, name);
- if (access(buf, R_OK) == 0) {
- trace_load_file(name, buf);
- return buf;
- }
- g_free(buf);
- }
- return NULL;
-}
-
-void qemu_add_data_dir(char *path)
-{
- int i;
+#define TRY_LOAD(path) \
+ do { \
+ ret = access(path, R_OK); \
+ trace_datadir_load_file(name, path, ret == 0 ? 0 : errno); \
+ if (ret == 0) { \
+ return maybepath; \
+ } \
+ g_clear_pointer(&path, g_free); \
+ } while (0)
- if (path == NULL) {
- return;
+ if (user_install_dir) {
+ maybepath = g_build_filename(user_install_dir, rel_install_dir,
+ name, NULL);
+ TRY_LOAD(maybepath);
}
- if (data_dir_idx == ARRAY_SIZE(data_dir)) {
- return;
- }
- for (i = 0; i < data_dir_idx; i++) {
- if (strcmp(data_dir[i], path) == 0) {
- g_free(path); /* duplicate */
- return;
+
+ if (in_build_dir) {
+ maybepath = g_build_filename(qemu_get_exec_dir(), rel_build_dir,
+ name, NULL);
+ } else {
+ if (extra_install_dirs) {
+ for (i = 0; extra_install_dirs[i] != NULL; i++) {
+ maybepath = g_build_filename(extra_install_dirs[i],
+ name, NULL);
+ TRY_LOAD(maybepath);
+ }
}
+
+ maybepath = g_build_filename(default_install_dir, rel_install_dir,
+ name, NULL);
}
- data_dir[data_dir_idx++] = path;
+ TRY_LOAD(maybepath);
+
+ return NULL;
}
-/*
- * 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)
+void qemu_set_user_data_dir(const char *path)
{
- 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);
+ g_free(user_data_dir);
+ user_data_dir = g_strdup(path);
}
void qemu_add_default_firmwarepath(void)
{
- char **dirs;
size_t i;
+ g_autofree char *builddir = NULL;
+
+ builddir = g_build_filename(qemu_get_exec_dir(), "pc-bios", NULL);
+ if (access(builddir, R_OK) == 0) {
+ in_build_dir = true;
+ }
/* add configured firmware directories */
- dirs = g_strsplit(CONFIG_QEMU_FIRMWAREPATH, G_SEARCHPATH_SEPARATOR_S, 0);
- for (i = 0; dirs[i] != NULL; i++) {
- qemu_add_data_dir(get_relocated_path(dirs[i]));
+ extra_firmware_dirs = g_strsplit(CONFIG_QEMU_FIRMWAREPATH,
+ G_SEARCHPATH_SEPARATOR_S, 0);
+ for (i = 0; extra_firmware_dirs[i] != NULL ; i++) {
+ g_autofree char *path = extra_firmware_dirs[i];
+ extra_firmware_dirs[i] = get_relocated_path(path);
}
- g_strfreev(dirs);
- /* try to find datadir relative to the executable path */
- qemu_add_data_dir(find_datadir());
+ /* Add default dirs relative to the executable path */
+ default_data_dir = get_relocated_path(CONFIG_QEMU_DATADIR);
+
+ trace_datadir_init(default_data_dir, in_build_dir);
}
void qemu_list_data_dirs(void)
{
int i;
- for (i = 0; i < data_dir_idx; i++) {
- printf("%s\n", data_dir[i]);
+ for (i = 0; extra_firmware_dirs[i] != NULL; i++) {
+ printf("%s\n", extra_firmware_dirs[i]);
}
+
+ printf("%s\n", default_data_dir);
}
diff --git a/softmmu/trace-events b/softmmu/trace-events
index 9c88887b3c..a9ba53f50d 100644
--- a/softmmu/trace-events
+++ b/softmmu/trace-events
@@ -4,6 +4,10 @@
# Since requests are raised via monitor, not many tracepoints are needed.
balloon_event(void *opaque, unsigned long addr) "opaque %p addr %lu"
+# datadir.c
+datadir_load_file(const char *name, const char *path, int err) "name %s location %s errno %d"
+datadir_init(const char *defdatadir, bool inbuilddir) "default data dir %s in build dir %d"
+
# ioport.c
cpu_in(unsigned int addr, char size, unsigned int val) "addr 0x%x(%c) value %u"
cpu_out(unsigned int addr, char size, unsigned int val) "addr 0x%x(%c) value %u"
@@ -26,7 +30,6 @@ vm_stop_flush_all(int ret) "ret %d"
# vl.c
vm_state_notify(int running, int reason, const char *reason_str) "running %d reason %d (%s)"
-load_file(const char *name, const char *path) "name %s location %s"
runstate_set(int current_state, const char *current_state_str, int new_state, const char *new_state_str) "current_run_state %d (%s) new_state %d (%s)"
system_wakeup_request(int reason) "reason=%d"
qemu_system_shutdown_request(int reason) "reason=%d"
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 54e920ada1..0a578255c7 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2898,7 +2898,7 @@ void qemu_init(int argc, char **argv, char **envp)
if (is_help_option(optarg)) {
list_data_dirs = true;
} else {
- qemu_add_data_dir(g_strdup(optarg));
+ qemu_set_user_data_dir(optarg);
}
break;
case QEMU_OPTION_bios:
diff --git a/tests/qtest/fuzz/fuzz.c b/tests/qtest/fuzz/fuzz.c
index 0ad4ba9e94..4438c7f141 100644
--- a/tests/qtest/fuzz/fuzz.c
+++ b/tests/qtest/fuzz/fuzz.c
@@ -186,7 +186,7 @@ int LLVMFuzzerInitialize(int *argc, char ***argv, char ***envp)
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);
+ qemu_set_data_dir(datadir);
} else {
g_free(datadir);
}
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 2/4] ui: move 'pc-bios/keymaps' to 'ui/keymaps'
2022-06-15 10:52 [PATCH v3 0/4] softmmu: make qemu_find_file more flexible wrt build dir layout Daniel P. Berrangé
2022-06-15 10:52 ` [PATCH v3 1/4] softmmu: rewrite handling of qemu_find_file Daniel P. Berrangé
@ 2022-06-15 10:52 ` Daniel P. Berrangé
2022-06-15 10:52 ` [PATCH v3 3/4] ui: find icons using qemu_find_file Daniel P. Berrangé
2022-06-15 10:52 ` [PATCH v3 4/4] net: convert to use qemu_find_file to locate bridge helper Daniel P. Berrangé
3 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2022-06-15 10:52 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, Bandan Das, Thomas Huth, Volker Rümelin,
Darren Kenny, Qiuhao Li, Jason Wang, Peter Maydell, Akihiko Odaki,
Stefan Hajnoczi, Laurent Vivier, Paolo Bonzini,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P. Berrangé
The 'keymaps' directory contents is nothing to do with the firmware
blobs. The 'pc-bios/keymaps' directory appears to have been used
previously as a convenience for getting the files installed into
a subdir of the firmware install dir, as well as to make it easier
to launch QEMU directly from the build tree. These requirements
do not need to be reflected in the source tree arrangement. The
keymaps logically belong with the UI code, and meson can install
them into the right place. For in-tree execution, we merely need
a suitable symlink from the source tree to the build tree.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
pc-bios/meson.build | 1 -
softmmu/datadir.c | 2 +-
{pc-bios => ui}/keymaps/ar | 0
{pc-bios => ui}/keymaps/bepo | 0
{pc-bios => ui}/keymaps/cz | 0
{pc-bios => ui}/keymaps/da | 0
{pc-bios => ui}/keymaps/de | 0
{pc-bios => ui}/keymaps/de-ch | 0
{pc-bios => ui}/keymaps/en-gb | 0
{pc-bios => ui}/keymaps/en-us | 0
{pc-bios => ui}/keymaps/es | 0
{pc-bios => ui}/keymaps/et | 0
{pc-bios => ui}/keymaps/fi | 0
{pc-bios => ui}/keymaps/fo | 0
{pc-bios => ui}/keymaps/fr | 0
{pc-bios => ui}/keymaps/fr-be | 0
{pc-bios => ui}/keymaps/fr-ca | 0
{pc-bios => ui}/keymaps/fr-ch | 0
{pc-bios => ui}/keymaps/hr | 0
{pc-bios => ui}/keymaps/hu | 0
{pc-bios => ui}/keymaps/is | 0
{pc-bios => ui}/keymaps/it | 0
{pc-bios => ui}/keymaps/ja | 0
{pc-bios => ui}/keymaps/lt | 0
{pc-bios => ui}/keymaps/lv | 0
{pc-bios => ui}/keymaps/meson.build | 0
{pc-bios => ui}/keymaps/mk | 0
{pc-bios => ui}/keymaps/nl | 0
{pc-bios => ui}/keymaps/no | 0
{pc-bios => ui}/keymaps/pl | 0
{pc-bios => ui}/keymaps/pt | 0
{pc-bios => ui}/keymaps/pt-br | 0
{pc-bios => ui}/keymaps/ru | 0
{pc-bios => ui}/keymaps/sl | 0
{pc-bios => ui}/keymaps/sv | 0
{pc-bios => ui}/keymaps/th | 0
{pc-bios => ui}/keymaps/tr | 0
ui/meson.build | 1 +
38 files changed, 2 insertions(+), 2 deletions(-)
rename {pc-bios => ui}/keymaps/ar (100%)
rename {pc-bios => ui}/keymaps/bepo (100%)
rename {pc-bios => ui}/keymaps/cz (100%)
rename {pc-bios => ui}/keymaps/da (100%)
rename {pc-bios => ui}/keymaps/de (100%)
rename {pc-bios => ui}/keymaps/de-ch (100%)
rename {pc-bios => ui}/keymaps/en-gb (100%)
rename {pc-bios => ui}/keymaps/en-us (100%)
rename {pc-bios => ui}/keymaps/es (100%)
rename {pc-bios => ui}/keymaps/et (100%)
rename {pc-bios => ui}/keymaps/fi (100%)
rename {pc-bios => ui}/keymaps/fo (100%)
rename {pc-bios => ui}/keymaps/fr (100%)
rename {pc-bios => ui}/keymaps/fr-be (100%)
rename {pc-bios => ui}/keymaps/fr-ca (100%)
rename {pc-bios => ui}/keymaps/fr-ch (100%)
rename {pc-bios => ui}/keymaps/hr (100%)
rename {pc-bios => ui}/keymaps/hu (100%)
rename {pc-bios => ui}/keymaps/is (100%)
rename {pc-bios => ui}/keymaps/it (100%)
rename {pc-bios => ui}/keymaps/ja (100%)
rename {pc-bios => ui}/keymaps/lt (100%)
rename {pc-bios => ui}/keymaps/lv (100%)
rename {pc-bios => ui}/keymaps/meson.build (100%)
rename {pc-bios => ui}/keymaps/mk (100%)
rename {pc-bios => ui}/keymaps/nl (100%)
rename {pc-bios => ui}/keymaps/no (100%)
rename {pc-bios => ui}/keymaps/pl (100%)
rename {pc-bios => ui}/keymaps/pt (100%)
rename {pc-bios => ui}/keymaps/pt-br (100%)
rename {pc-bios => ui}/keymaps/ru (100%)
rename {pc-bios => ui}/keymaps/sl (100%)
rename {pc-bios => ui}/keymaps/sv (100%)
rename {pc-bios => ui}/keymaps/th (100%)
rename {pc-bios => ui}/keymaps/tr (100%)
diff --git a/pc-bios/meson.build b/pc-bios/meson.build
index 41ba1c0ec7..e49c0e5f56 100644
--- a/pc-bios/meson.build
+++ b/pc-bios/meson.build
@@ -97,4 +97,3 @@ foreach f : blobs
endforeach
subdir('descriptors')
-subdir('keymaps')
diff --git a/softmmu/datadir.c b/softmmu/datadir.c
index 7457717542..32c765d228 100644
--- a/softmmu/datadir.c
+++ b/softmmu/datadir.c
@@ -62,7 +62,7 @@ char *qemu_find_file(int type, const char *name)
case QEMU_FILE_TYPE_KEYMAP:
user_install_dir = user_data_dir;
rel_install_dir = "keymaps";
- rel_build_dir = "pc-bios/keymaps";
+ rel_build_dir = "ui/keymaps";
default_install_dir = default_data_dir;
break;
diff --git a/pc-bios/keymaps/ar b/ui/keymaps/ar
similarity index 100%
rename from pc-bios/keymaps/ar
rename to ui/keymaps/ar
diff --git a/pc-bios/keymaps/bepo b/ui/keymaps/bepo
similarity index 100%
rename from pc-bios/keymaps/bepo
rename to ui/keymaps/bepo
diff --git a/pc-bios/keymaps/cz b/ui/keymaps/cz
similarity index 100%
rename from pc-bios/keymaps/cz
rename to ui/keymaps/cz
diff --git a/pc-bios/keymaps/da b/ui/keymaps/da
similarity index 100%
rename from pc-bios/keymaps/da
rename to ui/keymaps/da
diff --git a/pc-bios/keymaps/de b/ui/keymaps/de
similarity index 100%
rename from pc-bios/keymaps/de
rename to ui/keymaps/de
diff --git a/pc-bios/keymaps/de-ch b/ui/keymaps/de-ch
similarity index 100%
rename from pc-bios/keymaps/de-ch
rename to ui/keymaps/de-ch
diff --git a/pc-bios/keymaps/en-gb b/ui/keymaps/en-gb
similarity index 100%
rename from pc-bios/keymaps/en-gb
rename to ui/keymaps/en-gb
diff --git a/pc-bios/keymaps/en-us b/ui/keymaps/en-us
similarity index 100%
rename from pc-bios/keymaps/en-us
rename to ui/keymaps/en-us
diff --git a/pc-bios/keymaps/es b/ui/keymaps/es
similarity index 100%
rename from pc-bios/keymaps/es
rename to ui/keymaps/es
diff --git a/pc-bios/keymaps/et b/ui/keymaps/et
similarity index 100%
rename from pc-bios/keymaps/et
rename to ui/keymaps/et
diff --git a/pc-bios/keymaps/fi b/ui/keymaps/fi
similarity index 100%
rename from pc-bios/keymaps/fi
rename to ui/keymaps/fi
diff --git a/pc-bios/keymaps/fo b/ui/keymaps/fo
similarity index 100%
rename from pc-bios/keymaps/fo
rename to ui/keymaps/fo
diff --git a/pc-bios/keymaps/fr b/ui/keymaps/fr
similarity index 100%
rename from pc-bios/keymaps/fr
rename to ui/keymaps/fr
diff --git a/pc-bios/keymaps/fr-be b/ui/keymaps/fr-be
similarity index 100%
rename from pc-bios/keymaps/fr-be
rename to ui/keymaps/fr-be
diff --git a/pc-bios/keymaps/fr-ca b/ui/keymaps/fr-ca
similarity index 100%
rename from pc-bios/keymaps/fr-ca
rename to ui/keymaps/fr-ca
diff --git a/pc-bios/keymaps/fr-ch b/ui/keymaps/fr-ch
similarity index 100%
rename from pc-bios/keymaps/fr-ch
rename to ui/keymaps/fr-ch
diff --git a/pc-bios/keymaps/hr b/ui/keymaps/hr
similarity index 100%
rename from pc-bios/keymaps/hr
rename to ui/keymaps/hr
diff --git a/pc-bios/keymaps/hu b/ui/keymaps/hu
similarity index 100%
rename from pc-bios/keymaps/hu
rename to ui/keymaps/hu
diff --git a/pc-bios/keymaps/is b/ui/keymaps/is
similarity index 100%
rename from pc-bios/keymaps/is
rename to ui/keymaps/is
diff --git a/pc-bios/keymaps/it b/ui/keymaps/it
similarity index 100%
rename from pc-bios/keymaps/it
rename to ui/keymaps/it
diff --git a/pc-bios/keymaps/ja b/ui/keymaps/ja
similarity index 100%
rename from pc-bios/keymaps/ja
rename to ui/keymaps/ja
diff --git a/pc-bios/keymaps/lt b/ui/keymaps/lt
similarity index 100%
rename from pc-bios/keymaps/lt
rename to ui/keymaps/lt
diff --git a/pc-bios/keymaps/lv b/ui/keymaps/lv
similarity index 100%
rename from pc-bios/keymaps/lv
rename to ui/keymaps/lv
diff --git a/pc-bios/keymaps/meson.build b/ui/keymaps/meson.build
similarity index 100%
rename from pc-bios/keymaps/meson.build
rename to ui/keymaps/meson.build
diff --git a/pc-bios/keymaps/mk b/ui/keymaps/mk
similarity index 100%
rename from pc-bios/keymaps/mk
rename to ui/keymaps/mk
diff --git a/pc-bios/keymaps/nl b/ui/keymaps/nl
similarity index 100%
rename from pc-bios/keymaps/nl
rename to ui/keymaps/nl
diff --git a/pc-bios/keymaps/no b/ui/keymaps/no
similarity index 100%
rename from pc-bios/keymaps/no
rename to ui/keymaps/no
diff --git a/pc-bios/keymaps/pl b/ui/keymaps/pl
similarity index 100%
rename from pc-bios/keymaps/pl
rename to ui/keymaps/pl
diff --git a/pc-bios/keymaps/pt b/ui/keymaps/pt
similarity index 100%
rename from pc-bios/keymaps/pt
rename to ui/keymaps/pt
diff --git a/pc-bios/keymaps/pt-br b/ui/keymaps/pt-br
similarity index 100%
rename from pc-bios/keymaps/pt-br
rename to ui/keymaps/pt-br
diff --git a/pc-bios/keymaps/ru b/ui/keymaps/ru
similarity index 100%
rename from pc-bios/keymaps/ru
rename to ui/keymaps/ru
diff --git a/pc-bios/keymaps/sl b/ui/keymaps/sl
similarity index 100%
rename from pc-bios/keymaps/sl
rename to ui/keymaps/sl
diff --git a/pc-bios/keymaps/sv b/ui/keymaps/sv
similarity index 100%
rename from pc-bios/keymaps/sv
rename to ui/keymaps/sv
diff --git a/pc-bios/keymaps/th b/ui/keymaps/th
similarity index 100%
rename from pc-bios/keymaps/th
rename to ui/keymaps/th
diff --git a/pc-bios/keymaps/tr b/ui/keymaps/tr
similarity index 100%
rename from pc-bios/keymaps/tr
rename to ui/keymaps/tr
diff --git a/ui/meson.build b/ui/meson.build
index e9f48c5315..25c9a5ff8c 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -170,6 +170,7 @@ if have_system or xkbcommon.found()
endif
subdir('shader')
+subdir('keymaps')
if have_system
subdir('icons')
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 3/4] ui: find icons using qemu_find_file
2022-06-15 10:52 [PATCH v3 0/4] softmmu: make qemu_find_file more flexible wrt build dir layout Daniel P. Berrangé
2022-06-15 10:52 ` [PATCH v3 1/4] softmmu: rewrite handling of qemu_find_file Daniel P. Berrangé
2022-06-15 10:52 ` [PATCH v3 2/4] ui: move 'pc-bios/keymaps' to 'ui/keymaps' Daniel P. Berrangé
@ 2022-06-15 10:52 ` Daniel P. Berrangé
2022-06-15 10:52 ` [PATCH v3 4/4] net: convert to use qemu_find_file to locate bridge helper Daniel P. Berrangé
3 siblings, 0 replies; 8+ messages in thread
From: Daniel P. Berrangé @ 2022-06-15 10:52 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, Bandan Das, Thomas Huth, Volker Rümelin,
Darren Kenny, Qiuhao Li, Jason Wang, Peter Maydell, Akihiko Odaki,
Stefan Hajnoczi, Laurent Vivier, Paolo Bonzini,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P. Berrangé
The SDL/GTK/Cocoa UIs currently fail to load icons when run from the
build directory as get_resource returns a bogus path.
To address this we first re-arrange the ui/icons sub-directory
so that its layout reflects the contents that will be installed.
Then we introduce QEMU_FILE_TYPE_ICON to qemu_find_file such
that it can locate icons from the build dir.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
configure | 1 +
docs/conf.py | 4 +--
include/qemu/datadir.h | 2 ++
softmmu/datadir.c | 12 +++++++-
softmmu/trace-events | 2 +-
ui/cocoa.m | 3 +-
ui/gtk.c | 3 +-
.../apps/qemu.png} | Bin
.../{qemu_16x16.png => 16x16/apps/qemu.png} | Bin
.../{qemu_24x24.png => 24x24/apps/qemu.png} | Bin
.../apps/qemu.png} | Bin
.../{qemu_32x32.bmp => 32x32/apps/qemu.bmp} | Bin
.../{qemu_32x32.png => 32x32/apps/qemu.png} | Bin
.../{qemu_48x48.png => 48x48/apps/qemu.png} | Bin
.../apps/qemu.png} | Bin
.../{qemu_64x64.png => 64x64/apps/qemu.png} | Bin
ui/icons/meson.build | 27 ++++++++++++------
ui/icons/{ => scalable/apps}/qemu.svg | 0
ui/sdl2.c | 5 ++--
19 files changed, 43 insertions(+), 16 deletions(-)
rename ui/icons/{qemu_128x128.png => 128x128/apps/qemu.png} (100%)
rename ui/icons/{qemu_16x16.png => 16x16/apps/qemu.png} (100%)
rename ui/icons/{qemu_24x24.png => 24x24/apps/qemu.png} (100%)
rename ui/icons/{qemu_256x256.png => 256x256/apps/qemu.png} (100%)
rename ui/icons/{qemu_32x32.bmp => 32x32/apps/qemu.bmp} (100%)
rename ui/icons/{qemu_32x32.png => 32x32/apps/qemu.png} (100%)
rename ui/icons/{qemu_48x48.png => 48x48/apps/qemu.png} (100%)
rename ui/icons/{qemu_512x512.png => 512x512/apps/qemu.png} (100%)
rename ui/icons/{qemu_64x64.png => 64x64/apps/qemu.png} (100%)
rename ui/icons/{ => scalable/apps}/qemu.svg (100%)
diff --git a/configure b/configure
index 4b12a8094c..fdcbfbc1b1 100755
--- a/configure
+++ b/configure
@@ -2218,6 +2218,7 @@ LINKS="$LINKS tests/avocado tests/data"
LINKS="$LINKS tests/qemu-iotests/check"
LINKS="$LINKS python"
LINKS="$LINKS contrib/plugins/Makefile "
+LINKS="$LINKS ui/icons "
for f in $LINKS ; do
if [ -e "$source_path/$f" ]; then
mkdir -p `dirname ./$f`
diff --git a/docs/conf.py b/docs/conf.py
index 49dab44cca..16d5d96228 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -180,9 +180,9 @@
"navigation_with_keys": True,
}
-html_logo = os.path.join(qemu_docdir, "../ui/icons/qemu_128x128.png")
+html_logo = os.path.join(qemu_docdir, "../ui/icons/128x128/apps/qemu.png")
-html_favicon = os.path.join(qemu_docdir, "../ui/icons/qemu_32x32.png")
+html_favicon = os.path.join(qemu_docdir, "../ui/icons/32x32/apps/qemu.png")
# Add any paths that contain custom static files (such as style sheets) here,
# relative to this directory. They are copied after the builtin static files,
diff --git a/include/qemu/datadir.h b/include/qemu/datadir.h
index a333cd9b0d..427e90787a 100644
--- a/include/qemu/datadir.h
+++ b/include/qemu/datadir.h
@@ -3,6 +3,8 @@
#define QEMU_FILE_TYPE_BIOS 0
#define QEMU_FILE_TYPE_KEYMAP 1
+#define QEMU_FILE_TYPE_ICON 2
+
/**
* qemu_find_file:
* @type: QEMU_FILE_TYPE_BIOS (for BIOS, VGA BIOS)
diff --git a/softmmu/datadir.c b/softmmu/datadir.c
index 32c765d228..e5d1fd0116 100644
--- a/softmmu/datadir.c
+++ b/softmmu/datadir.c
@@ -35,6 +35,7 @@ static char **extra_firmware_dirs;
/* Default built-in directories */
static char *default_data_dir;
+static char *default_icon_dir;
/* Whether we're known to be executing from a build tree */
static bool in_build_dir;
@@ -66,6 +67,12 @@ char *qemu_find_file(int type, const char *name)
default_install_dir = default_data_dir;
break;
+ case QEMU_FILE_TYPE_ICON:
+ rel_install_dir = "hicolor";
+ rel_build_dir = "ui/icons";
+ default_install_dir = default_icon_dir;
+ break;
+
default:
abort();
}
@@ -132,8 +139,11 @@ void qemu_add_default_firmwarepath(void)
/* Add default dirs relative to the executable path */
default_data_dir = get_relocated_path(CONFIG_QEMU_DATADIR);
+ default_icon_dir = get_relocated_path(CONFIG_QEMU_ICONDIR);
- trace_datadir_init(default_data_dir, in_build_dir);
+ trace_datadir_init(default_data_dir,
+ default_icon_dir,
+ in_build_dir);
}
void qemu_list_data_dirs(void)
diff --git a/softmmu/trace-events b/softmmu/trace-events
index a9ba53f50d..9c00e9f389 100644
--- a/softmmu/trace-events
+++ b/softmmu/trace-events
@@ -6,7 +6,7 @@ balloon_event(void *opaque, unsigned long addr) "opaque %p addr %lu"
# datadir.c
datadir_load_file(const char *name, const char *path, int err) "name %s location %s errno %d"
-datadir_init(const char *defdatadir, bool inbuilddir) "default data dir %s in build dir %d"
+datadir_init(const char *defdatadir, const char *deficondir, bool inbuilddir) "default data dir %s icon dir %s in build dir %d"
# ioport.c
cpu_in(unsigned int addr, char size, unsigned int val) "addr 0x%x(%c) value %u"
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 09a62817f2..c906e618f6 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -45,6 +45,7 @@
#include "qemu/cutils.h"
#include "qemu/main-loop.h"
#include "qemu/module.h"
+#include "qemu/datadir.h"
#include <Carbon/Carbon.h>
#include "hw/core/cpu.h"
@@ -1558,7 +1559,7 @@ - (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");
+ char *icon_path_c = qemu_find_file(QEMU_FILE_TYPE_ICON, "512x512/apps/qemu.png");
NSString *icon_path = [NSString stringWithUTF8String:icon_path_c];
g_free(icon_path_c);
NSImage *icon = [[NSImage alloc] initWithContentsOfFile:icon_path];
diff --git a/ui/gtk.c b/ui/gtk.c
index c57c36749e..7f798a0560 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -31,6 +31,7 @@
#define LOCALEDIR "po"
#include "qemu/osdep.h"
+#include "qemu/datadir.h"
#include "qapi/error.h"
#include "qapi/qapi-commands-control.h"
#include "qapi/qapi-commands-machine.h"
@@ -2314,7 +2315,7 @@ 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 = qemu_find_file(QEMU_FILE_TYPE_ICON, NULL);
gtk_icon_theme_prepend_search_path(theme, dir);
g_free(dir);
g_set_prgname("qemu");
diff --git a/ui/icons/qemu_128x128.png b/ui/icons/128x128/apps/qemu.png
similarity index 100%
rename from ui/icons/qemu_128x128.png
rename to ui/icons/128x128/apps/qemu.png
diff --git a/ui/icons/qemu_16x16.png b/ui/icons/16x16/apps/qemu.png
similarity index 100%
rename from ui/icons/qemu_16x16.png
rename to ui/icons/16x16/apps/qemu.png
diff --git a/ui/icons/qemu_24x24.png b/ui/icons/24x24/apps/qemu.png
similarity index 100%
rename from ui/icons/qemu_24x24.png
rename to ui/icons/24x24/apps/qemu.png
diff --git a/ui/icons/qemu_256x256.png b/ui/icons/256x256/apps/qemu.png
similarity index 100%
rename from ui/icons/qemu_256x256.png
rename to ui/icons/256x256/apps/qemu.png
diff --git a/ui/icons/qemu_32x32.bmp b/ui/icons/32x32/apps/qemu.bmp
similarity index 100%
rename from ui/icons/qemu_32x32.bmp
rename to ui/icons/32x32/apps/qemu.bmp
diff --git a/ui/icons/qemu_32x32.png b/ui/icons/32x32/apps/qemu.png
similarity index 100%
rename from ui/icons/qemu_32x32.png
rename to ui/icons/32x32/apps/qemu.png
diff --git a/ui/icons/qemu_48x48.png b/ui/icons/48x48/apps/qemu.png
similarity index 100%
rename from ui/icons/qemu_48x48.png
rename to ui/icons/48x48/apps/qemu.png
diff --git a/ui/icons/qemu_512x512.png b/ui/icons/512x512/apps/qemu.png
similarity index 100%
rename from ui/icons/qemu_512x512.png
rename to ui/icons/512x512/apps/qemu.png
diff --git a/ui/icons/qemu_64x64.png b/ui/icons/64x64/apps/qemu.png
similarity index 100%
rename from ui/icons/qemu_64x64.png
rename to ui/icons/64x64/apps/qemu.png
diff --git a/ui/icons/meson.build b/ui/icons/meson.build
index 12c52080eb..ac9111260b 100644
--- a/ui/icons/meson.build
+++ b/ui/icons/meson.build
@@ -1,13 +1,24 @@
-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')
+
+png_icon_sizes = [
+ '16x16',
+ '24x24',
+ '32x32',
+ '48x48',
+ '64x64',
+ '128x128',
+ '256x256',
+ '512x512',
+]
+
+foreach icon_size: png_icon_sizes
+ install_data(
+ join_paths(icon_size, 'apps', 'qemu.png'),
+ install_dir: join_paths(qemu_icondir, 'hicolor', icon_size, 'apps')
+ )
endforeach
-install_data('qemu_32x32.bmp',
- rename: 'qemu.bmp',
+install_data('32x32/apps/qemu.bmp',
install_dir: qemu_icondir / 'hicolor' / '32x32' / 'apps')
-install_data('qemu.svg',
+install_data('scalable/apps/qemu.svg',
install_dir: qemu_icondir / 'hicolor' / 'scalable' / 'apps')
diff --git a/ui/icons/qemu.svg b/ui/icons/scalable/apps/qemu.svg
similarity index 100%
rename from ui/icons/qemu.svg
rename to ui/icons/scalable/apps/qemu.svg
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 8cb77416af..64435221cd 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -26,6 +26,7 @@
#include "qemu/osdep.h"
#include "qemu/module.h"
#include "qemu/cutils.h"
+#include "qemu/datadir.h"
#include "ui/console.h"
#include "ui/input.h"
#include "ui/sdl2.h"
@@ -910,11 +911,11 @@ 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");
+ dir = qemu_find_file(QEMU_FILE_TYPE_ICON, "128x128/apps/qemu.png");
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");
+ dir = qemu_find_file(QEMU_FILE_TYPE_ICON, "32x32/apps/qemu.bmp");
icon = SDL_LoadBMP(dir);
if (icon) {
uint32_t colorkey = SDL_MapRGB(icon->format, 255, 255, 255);
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH v3 4/4] net: convert to use qemu_find_file to locate bridge helper
2022-06-15 10:52 [PATCH v3 0/4] softmmu: make qemu_find_file more flexible wrt build dir layout Daniel P. Berrangé
` (2 preceding siblings ...)
2022-06-15 10:52 ` [PATCH v3 3/4] ui: find icons using qemu_find_file Daniel P. Berrangé
@ 2022-06-15 10:52 ` Daniel P. Berrangé
2022-06-15 11:42 ` Paolo Bonzini
3 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2022-06-15 10:52 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, Bandan Das, Thomas Huth, Volker Rümelin,
Darren Kenny, Qiuhao Li, Jason Wang, Peter Maydell, Akihiko Odaki,
Stefan Hajnoczi, Laurent Vivier, Paolo Bonzini,
Philippe Mathieu-Daudé, Gerd Hoffmann,
Daniel P. Berrangé
The TAP device code currently uses get_relocate_path to find the bridge
helper, however, this fails when run from the build dir. Adding support
to qemu_find_file for helper binaries, allows it to work from both the
(relocated) install tree and build dir.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/net/net.h | 3 ++-
include/qemu/datadir.h | 1 +
net/tap.c | 5 ++++-
qemu-options.hx | 4 ++--
softmmu/datadir.c | 9 +++++++++
softmmu/trace-events | 2 +-
6 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/include/net/net.h b/include/net/net.h
index 523136c7ac..6a853512ac 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -228,7 +228,8 @@ 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_BRIDGE_HELPER "qemu-bridge-helper"
+#define DEFAULT_BRIDGE_HELPER_PATH CONFIG_QEMU_HELPERDIR "/qemu-bridge-helper"
#define DEFAULT_BRIDGE_INTERFACE "br0"
void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd);
diff --git a/include/qemu/datadir.h b/include/qemu/datadir.h
index 427e90787a..a211b6b235 100644
--- a/include/qemu/datadir.h
+++ b/include/qemu/datadir.h
@@ -4,6 +4,7 @@
#define QEMU_FILE_TYPE_BIOS 0
#define QEMU_FILE_TYPE_KEYMAP 1
#define QEMU_FILE_TYPE_ICON 2
+#define QEMU_FILE_TYPE_HELPER 3
/**
* qemu_find_file:
diff --git a/net/tap.c b/net/tap.c
index b3ddfd4a74..161608e34a 100644
--- a/net/tap.c
+++ b/net/tap.c
@@ -42,6 +42,7 @@
#include "qemu/error-report.h"
#include "qemu/main-loop.h"
#include "qemu/sockets.h"
+#include "qemu/datadir.h"
#include "net/tap.h"
@@ -507,9 +508,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 = qemu_find_file(QEMU_FILE_TYPE_HELPER,
+ DEFAULT_BRIDGE_HELPER);
}
+ g_printerr("Helper %s\n", helper);
if (socketpair(PF_UNIX, SOCK_STREAM, 0, sv) == -1) {
error_setg_errno(errp, errno, "socketpair() failed");
return -1;
diff --git a/qemu-options.hx b/qemu-options.hx
index 377d22fbd8..b5b7e75048 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_BRIDGE_HELPER_PATH ") 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_BRIDGE_HELPER_PATH ")\n"
#endif
#ifdef __linux__
"-netdev l2tpv3,id=str,src=srcaddr,dst=dstaddr[,srcport=srcport][,dstport=dstport]\n"
diff --git a/softmmu/datadir.c b/softmmu/datadir.c
index e5d1fd0116..a68fe7167a 100644
--- a/softmmu/datadir.c
+++ b/softmmu/datadir.c
@@ -36,6 +36,7 @@ static char **extra_firmware_dirs;
/* Default built-in directories */
static char *default_data_dir;
static char *default_icon_dir;
+static char *default_helper_dir;
/* Whether we're known to be executing from a build tree */
static bool in_build_dir;
@@ -73,6 +74,12 @@ char *qemu_find_file(int type, const char *name)
default_install_dir = default_icon_dir;
break;
+ case QEMU_FILE_TYPE_HELPER:
+ rel_install_dir = "";
+ rel_build_dir = "";
+ default_install_dir = default_helper_dir;
+ break;
+
default:
abort();
}
@@ -140,9 +147,11 @@ void qemu_add_default_firmwarepath(void)
/* Add default dirs relative to the executable path */
default_data_dir = get_relocated_path(CONFIG_QEMU_DATADIR);
default_icon_dir = get_relocated_path(CONFIG_QEMU_ICONDIR);
+ default_helper_dir = get_relocated_path(CONFIG_QEMU_HELPERDIR);
trace_datadir_init(default_data_dir,
default_icon_dir,
+ default_helper_dir,
in_build_dir);
}
diff --git a/softmmu/trace-events b/softmmu/trace-events
index 9c00e9f389..b22d7e7714 100644
--- a/softmmu/trace-events
+++ b/softmmu/trace-events
@@ -6,7 +6,7 @@ balloon_event(void *opaque, unsigned long addr) "opaque %p addr %lu"
# datadir.c
datadir_load_file(const char *name, const char *path, int err) "name %s location %s errno %d"
-datadir_init(const char *defdatadir, const char *deficondir, bool inbuilddir) "default data dir %s icon dir %s in build dir %d"
+datadir_init(const char *defdatadir, const char *deficondir, const char *defhelperdir, bool inbuilddir) "default data dir %s icon dir %s helper dir %s in build dir %d"
# ioport.c
cpu_in(unsigned int addr, char size, unsigned int val) "addr 0x%x(%c) value %u"
--
2.36.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v3 4/4] net: convert to use qemu_find_file to locate bridge helper
2022-06-15 10:52 ` [PATCH v3 4/4] net: convert to use qemu_find_file to locate bridge helper Daniel P. Berrangé
@ 2022-06-15 11:42 ` Paolo Bonzini
2022-06-15 12:04 ` Daniel P. Berrangé
0 siblings, 1 reply; 8+ messages in thread
From: Paolo Bonzini @ 2022-06-15 11:42 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Alexander Bulekov, Bandan Das, Thomas Huth, Volker Rümelin,
Darren Kenny, Qiuhao Li, Jason Wang, Peter Maydell, Akihiko Odaki,
Stefan Hajnoczi, Laurent Vivier, Philippe Mathieu-Daudé,
Gerd Hoffmann
On 6/15/22 12:52, Daniel P. Berrangé wrote:
>
> + case QEMU_FILE_TYPE_HELPER:
> + rel_install_dir = "";
> + rel_build_dir = "";
> + default_install_dir = default_helper_dir;
> + break;
> +
You're replacing ad hoc rules in Akihiko's meson.build with an ad hoc
enum + the corresponding "case"s here in qemu_find_file(). There is
duplication anyway, in this case between Meson and QEMU (plus QEMU needs
to know about two filesystem layouts).
Paolo
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 4/4] net: convert to use qemu_find_file to locate bridge helper
2022-06-15 11:42 ` Paolo Bonzini
@ 2022-06-15 12:04 ` Daniel P. Berrangé
2022-06-15 16:47 ` Akihiko Odaki
0 siblings, 1 reply; 8+ messages in thread
From: Daniel P. Berrangé @ 2022-06-15 12:04 UTC (permalink / raw)
To: Paolo Bonzini
Cc: qemu-devel, Alexander Bulekov, Bandan Das, Thomas Huth,
Volker Rümelin, Darren Kenny, Qiuhao Li, Jason Wang,
Peter Maydell, Akihiko Odaki, Stefan Hajnoczi, Laurent Vivier,
Philippe Mathieu-Daudé, Gerd Hoffmann
On Wed, Jun 15, 2022 at 01:42:58PM +0200, Paolo Bonzini wrote:
> On 6/15/22 12:52, Daniel P. Berrangé wrote:
> > + case QEMU_FILE_TYPE_HELPER:
> > + rel_install_dir = "";
> > + rel_build_dir = "";
> > + default_install_dir = default_helper_dir;
> > + break;
> > +
>
> You're replacing ad hoc rules in Akihiko's meson.build with an ad hoc enum +
> the corresponding "case"s here in qemu_find_file(). There is duplication
> anyway, in this case between Meson and QEMU (plus QEMU needs to know about
> two filesystem layouts).
IMHO this is simpler to deal with than the meson additions, and also
avoids the confusion of having files appearing in two places in the
build dir.
If we really want to have the build dir look just like the install
dir though, why write custom meson commands per file type at all,
instead add a rule that always invokes
DESTDIR=$(BUILDDIR)/vroot ninja install
to populate a dir that's guaranteed identical to the install layout
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] 8+ messages in thread
* Re: [PATCH v3 4/4] net: convert to use qemu_find_file to locate bridge helper
2022-06-15 12:04 ` Daniel P. Berrangé
@ 2022-06-15 16:47 ` Akihiko Odaki
0 siblings, 0 replies; 8+ messages in thread
From: Akihiko Odaki @ 2022-06-15 16:47 UTC (permalink / raw)
To: Daniel P. Berrangé, Paolo Bonzini
Cc: qemu-devel, Alexander Bulekov, Bandan Das, Thomas Huth,
Volker Rümelin, Darren Kenny, Qiuhao Li, Jason Wang,
Peter Maydell, Stefan Hajnoczi, Laurent Vivier,
Philippe Mathieu-Daudé, Gerd Hoffmann
On 2022/06/15 21:04, Daniel P. Berrangé wrote:
> On Wed, Jun 15, 2022 at 01:42:58PM +0200, Paolo Bonzini wrote:
>> On 6/15/22 12:52, Daniel P. Berrangé wrote:
>>> + case QEMU_FILE_TYPE_HELPER:
>>> + rel_install_dir = "";
>>> + rel_build_dir = "";
>>> + default_install_dir = default_helper_dir;
>>> + break;
>>> +
>>
>> You're replacing ad hoc rules in Akihiko's meson.build with an ad hoc enum +
>> the corresponding "case"s here in qemu_find_file(). There is duplication
>> anyway, in this case between Meson and QEMU (plus QEMU needs to know about
>> two filesystem layouts).
>
> IMHO this is simpler to deal with than the meson additions, and also
> avoids the confusion of having files appearing in two places in the
> build dir.
Thanks to Paolo's suggestion to unify the common code to build the
bundle tree, the required code for each bundled file is just a statement
now (something like: bundles += { destination: source }) in the v6.
Doing everything in Meson also allows to reuse the knowledge of the
build tree Meson already has. I do no longer think my patch series are
complicated more than yours. It even has less lines now.
There is still a room for improvements though. Particularly, the
installing code and bundle-tree code are still duplicate. For example,
pc-bios/meson.build now has the following code:
install_data(blobs, install_dir: qemu_datadir)
foreach f : blobs
bundles += { qemu_datadir / f: meson.current_source_dir() / f }
endforeach
It would be nice if it can be written like:
foreach f : blobs
bundle_data(qemu_datadir / f, f)
endforeach
Unfortunately Meson does not allow this.
Another problem is that the top-level meson.build is somewhat clutter.
In my opinion, it is a persistent problem of the build system but I
don't have a solution.
Anyway, I think my patch series is as close to the ideal as Meson
currently allows.
The confusion caused by the files appearing in two places in the
build tree should be minimal. qemu-bundle is implemented entirely with
symbolic links. If you know what is a symbolic link, you also know it is
an alias and the files appear in different places, and I expect everyone
hacking QEMU knows symbolic link.
>
> If we really want to have the build dir look just like the install
> dir though, why write custom meson commands per file type at all,
> instead add a rule that always invokes
>
> DESTDIR=$(BUILDDIR)/vroot ninja install
>
> to populate a dir that's guaranteed identical to the install layout
Unfortunately Meson cannot define a rule which will be always invoked as
far as I know.
Regards,
Akihiko Odaki
>
> Regards,
> Daniel
^ permalink raw reply [flat|nested] 8+ messages in thread
