* [RFC PATCH] os-posix: fix regression for install-less datadir location
@ 2020-07-16 14:11 Marc-André Lureau
2020-07-16 14:15 ` Marc-André Lureau
2020-08-18 9:10 ` Paolo Bonzini
0 siblings, 2 replies; 5+ messages in thread
From: Marc-André Lureau @ 2020-07-16 14:11 UTC (permalink / raw)
To: qemu-devel; +Cc: Marc-André Lureau, joe.slater, Paolo Bonzini
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] os-posix: fix regression for install-less datadir location
2020-07-16 14:11 [RFC PATCH] os-posix: fix regression for install-less datadir location Marc-André Lureau
@ 2020-07-16 14:15 ` Marc-André Lureau
2020-08-18 9:10 ` Paolo Bonzini
1 sibling, 0 replies; 5+ messages in thread
From: Marc-André Lureau @ 2020-07-16 14:15 UTC (permalink / raw)
To: QEMU; +Cc: Paolo Bonzini, Joe Slater
[-- Attachment #1: Type: text/plain, Size: 2991 bytes --]
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
[-- Attachment #2: Type: text/html, Size: 4043 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] os-posix: fix regression for install-less datadir location
2020-07-16 14:11 [RFC PATCH] os-posix: fix regression for install-less datadir location Marc-André Lureau
2020-07-16 14:15 ` Marc-André Lureau
@ 2020-08-18 9:10 ` Paolo Bonzini
2020-08-18 13:10 ` Peter Maydell
1 sibling, 1 reply; 5+ messages in thread
From: Paolo Bonzini @ 2020-08-18 9:10 UTC (permalink / raw)
To: Marc-André Lureau, qemu-devel; +Cc: joe.slater
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] os-posix: fix regression for install-less datadir location
2020-08-18 9:10 ` Paolo Bonzini
@ 2020-08-18 13:10 ` Peter Maydell
2020-08-18 16:23 ` Paolo Bonzini
0 siblings, 1 reply; 5+ messages in thread
From: Peter Maydell @ 2020-08-18 13:10 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Marc-André Lureau, Joe Slater, QEMU Developers
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH] os-posix: fix regression for install-less datadir location
2020-08-18 13:10 ` Peter Maydell
@ 2020-08-18 16:23 ` Paolo Bonzini
0 siblings, 0 replies; 5+ messages in thread
From: Paolo Bonzini @ 2020-08-18 16:23 UTC (permalink / raw)
To: Peter Maydell; +Cc: Marc-André Lureau, Joe Slater, QEMU Developers
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-18 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-16 14:11 [RFC PATCH] os-posix: fix regression for install-less datadir location Marc-André Lureau
2020-07-16 14:15 ` Marc-André Lureau
2020-08-18 9:10 ` Paolo Bonzini
2020-08-18 13:10 ` Peter Maydell
2020-08-18 16:23 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.