os_find_datadir() used to check the ../share/qemu location (regardless
of CONFIG_QEMU_DATADIR). It turns out that people rely on that location
for running qemu in an arbitrary "install-less/portable" fashion. Change
the logic to return that directory as a last resort.
(this is an alternative to the patch "[PATCH 1/1] os_find_datadir: search
as in version 4.2" from Joe Slater)
Fixes: 6dd2dacedd83d12328 ("os-posix: simplify os_find_datadir")
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
os-posix.c | 15 ++++++++++++---
scripts/create_config | 2 +-
2 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/os-posix.c b/os-posix.c
index b674b20b1b1..bd0ed0c14d1 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -82,8 +82,13 @@ void os_setup_signal_handling(void)
/*
* 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.
+ *
+ * If running from the install location (CONFIG_BINDIR), this will be
+ * CONFIG_QEMU_DATADIR.
+ *
+ * Otherwise, fallback on "$execdir/../pc-bios" if it exists (the build tree
+ * location), else on "$execdir/../share/qemu" (for the install-less/"portable"
+ * version).
*/
char *os_find_datadir(void)
{
@@ -93,12 +98,16 @@ char *os_find_datadir(void)
exec_dir = qemu_get_exec_dir();
g_return_val_if_fail(exec_dir != NULL, NULL);
+ if (g_str_has_prefix(exec_dir, CONFIG_BINDIR)) {
+ return g_strdup(CONFIG_QEMU_DATADIR);
+ }
+
dir = g_build_filename(exec_dir, "..", "pc-bios", NULL);
if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
return g_steal_pointer(&dir);
}
- return g_strdup(CONFIG_QEMU_DATADIR);
+ return g_build_filename(exec_dir, "..", "share", "qemu", NULL);
}
void os_set_proc_name(const char *s)
diff --git a/scripts/create_config b/scripts/create_config
index 6d8f08b39da..03f8cb1dc10 100755
--- a/scripts/create_config
+++ b/scripts/create_config
@@ -15,7 +15,7 @@ case $line in
echo "#define QEMU_VERSION_MINOR $minor"
echo "#define QEMU_VERSION_MICRO $micro"
;;
- qemu_*dir=* | qemu_*path=*) # qemu-specific directory configuration
+ bindir=* | qemu_*dir=* | qemu_*path=*) # qemu-specific directory configuration
name=${line%=*}
value=${line#*=}
define_name=$(echo $name | LC_ALL=C tr '[a-z]' '[A-Z]')
--
2.27.0.221.ga08a83db2b
Hi On Thu, Jul 16, 2020 at 6:11 PM Marc-André Lureau < marcandre.lureau@redhat.com> wrote: > os_find_datadir() used to check the ../share/qemu location (regardless > of CONFIG_QEMU_DATADIR). It turns out that people rely on that location > for running qemu in an arbitrary "install-less/portable" fashion. Change > the logic to return that directory as a last resort. > > (this is an alternative to the patch "[PATCH 1/1] os_find_datadir: search > as in version 4.2" from Joe Slater) > > Fixes: 6dd2dacedd83d12328 ("os-posix: simplify os_find_datadir") > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > os-posix.c | 15 ++++++++++++--- > scripts/create_config | 2 +- > 2 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/os-posix.c b/os-posix.c > index b674b20b1b1..bd0ed0c14d1 100644 > --- a/os-posix.c > +++ b/os-posix.c > @@ -82,8 +82,13 @@ void os_setup_signal_handling(void) > > /* > * 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. > + * > + * If running from the install location (CONFIG_BINDIR), this will be > + * CONFIG_QEMU_DATADIR. > + * > + * Otherwise, fallback on "$execdir/../pc-bios" if it exists (the build > tree > + * location), else on "$execdir/../share/qemu" (for the > install-less/"portable" > + * version). > */ > char *os_find_datadir(void) > { > @@ -93,12 +98,16 @@ char *os_find_datadir(void) > exec_dir = qemu_get_exec_dir(); > g_return_val_if_fail(exec_dir != NULL, NULL); > > + if (g_str_has_prefix(exec_dir, CONFIG_BINDIR)) { > I realize g_str_has_prefix() may not be good enough (it was meant to ignore the / suffix..). I guess we can just go with the version from Joe instead. + return g_strdup(CONFIG_QEMU_DATADIR); > + } > + > dir = g_build_filename(exec_dir, "..", "pc-bios", NULL); > if (g_file_test(dir, G_FILE_TEST_IS_DIR)) { > return g_steal_pointer(&dir); > } > > - return g_strdup(CONFIG_QEMU_DATADIR); > + return g_build_filename(exec_dir, "..", "share", "qemu", NULL); > } > > void os_set_proc_name(const char *s) > diff --git a/scripts/create_config b/scripts/create_config > index 6d8f08b39da..03f8cb1dc10 100755 > --- a/scripts/create_config > +++ b/scripts/create_config > @@ -15,7 +15,7 @@ case $line in > echo "#define QEMU_VERSION_MINOR $minor" > echo "#define QEMU_VERSION_MICRO $micro" > ;; > - qemu_*dir=* | qemu_*path=*) # qemu-specific directory configuration > + bindir=* | qemu_*dir=* | qemu_*path=*) # qemu-specific directory > configuration > name=${line%=*} > value=${line#*=} > define_name=$(echo $name | LC_ALL=C tr '[a-z]' '[A-Z]') > -- > 2.27.0.221.ga08a83db2b > > > -- Marc-André Lureau
On 16/07/20 16:11, Marc-André Lureau wrote: > os_find_datadir() used to check the ../share/qemu location (regardless > of CONFIG_QEMU_DATADIR). It turns out that people rely on that location > for running qemu in an arbitrary "install-less/portable" fashion. Change > the logic to return that directory as a last resort. > > (this is an alternative to the patch "[PATCH 1/1] os_find_datadir: search > as in version 4.2" from Joe Slater) > > Fixes: 6dd2dacedd83d12328 ("os-posix: simplify os_find_datadir") > Cc: Paolo Bonzini <pbonzini@redhat.com> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> For 5.2 I plan to support fully relocatable installs, so I think this will not be needed. The idea is to write a function like char *get_relocatable_path(const char *dir); That takes CONFIG_QEMU_*DIR as the argument, turns it into a path relative to bindir, and tacks it to the end of qemu_get_exec_dir(). So for example all references to CONFIG_QEMU_DATADIR would invoke get_relocatable_path(CONFIG_QEMU_DATADIR), which would return something like "/usr/bin/../share/qemu". Paolo
On Tue, 18 Aug 2020 at 10:11, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 16/07/20 16:11, Marc-André Lureau wrote: > > os_find_datadir() used to check the ../share/qemu location (regardless > > of CONFIG_QEMU_DATADIR). It turns out that people rely on that location > > for running qemu in an arbitrary "install-less/portable" fashion. Change > > the logic to return that directory as a last resort. > > > > (this is an alternative to the patch "[PATCH 1/1] os_find_datadir: search > > as in version 4.2" from Joe Slater) > > > > Fixes: 6dd2dacedd83d12328 ("os-posix: simplify os_find_datadir") > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > For 5.2 I plan to support fully relocatable installs, so I think this > will not be needed. > > The idea is to write a function like > > char *get_relocatable_path(const char *dir); > > That takes CONFIG_QEMU_*DIR as the argument, turns it into a path > relative to bindir, and tacks it to the end of qemu_get_exec_dir(). > > So for example all references to CONFIG_QEMU_DATADIR would invoke > get_relocatable_path(CONFIG_QEMU_DATADIR), which would return something > like "/usr/bin/../share/qemu". Unless you have that series ready-to-roll, I think it would be useful to just fix the regression (and cc qemu-stable on it so it gets backported to 5.1.1) for the moment. thanks -- PMM
On 18/08/20 15:10, Peter Maydell wrote: >> So for example all references to CONFIG_QEMU_DATADIR would invoke >> get_relocatable_path(CONFIG_QEMU_DATADIR), which would return something >> like "/usr/bin/../share/qemu". > Unless you have that series ready-to-roll, I think it would > be useful to just fix the regression (and cc qemu-stable on it > so it gets backported to 5.1.1) for the moment. I have it ready but I agree it's not suitable for backporting (among other things, I've never tested it on the non-meson build system). I can rebase it on top of this patch with no issue. Paolo
© 2016 - 2024 Red Hat, Inc.