qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] os_find_datadir:  search as in version 4.2
@ 2020-06-15 22:58 Joe Slater
  2020-06-16  9:19 ` Peter Maydell
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Joe Slater @ 2020-06-15 22:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: joe.slater, randy.macleod

Always look for ../share/qemu then ../pc-bios when looking for datadir.

Signed-off-by: Joe Slater <joe.slater@windriver.com>
---
 os-posix.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/os-posix.c b/os-posix.c
index 3cd52e1e70..f77da94bf6 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -82,8 +82,9 @@ void os_setup_signal_handling(void)
 
 /*
  * Find a likely location for support files using the location of the binary.
+ * Typically, this would be "$bindir/../share/qemu".
  * When running from the build tree this will be "$bindir/../pc-bios".
- * Otherwise, this is CONFIG_QEMU_DATADIR.
+ * Otherwise, this is CONFIG_QEMU_DATADIR as constructed by configure.
  */
 char *os_find_datadir(void)
 {
@@ -93,6 +94,12 @@ char *os_find_datadir(void)
     exec_dir = qemu_get_exec_dir();
     g_return_val_if_fail(exec_dir != NULL, NULL);
 
+    dir = g_build_filename(exec_dir, "..", "share", "qemu", NULL);
+    if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
+        return g_steal_pointer(&dir);
+    }
+    g_free(dir);  /* no autofree this time */
+
     dir = g_build_filename(exec_dir, "..", "pc-bios", NULL);
     if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
         return g_steal_pointer(&dir);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-06-15 22:58 [PATCH 1/1] os_find_datadir: search as in version 4.2 Joe Slater
@ 2020-06-16  9:19 ` Peter Maydell
  2020-06-16 15:37   ` Slater, Joseph
  2020-07-15 19:37   ` Peter Maydell
  2020-07-16 14:12 ` Marc-André Lureau
  2020-08-10  7:33 ` Marc-André Lureau
  2 siblings, 2 replies; 13+ messages in thread
From: Peter Maydell @ 2020-06-16  9:19 UTC (permalink / raw)
  To: Joe Slater
  Cc: Marc-André Lureau, Paolo Bonzini, QEMU Developers, MacLeod, Randy

On Tue, 16 Jun 2020 at 00:00, Joe Slater <joe.slater@windriver.com> wrote:
>
> Always look for ../share/qemu then ../pc-bios when looking for datadir.

Could you provide some more context, please? Why is this
change useful; presumably we broke some setup in 5.0, but
what exactly ?

I'm guessing this might be a regression introduced by commit
6dd2dacedd83d12328 so I'm ccing the relevant people.

> Signed-off-by: Joe Slater <joe.slater@windriver.com>
> ---
>  os-posix.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/os-posix.c b/os-posix.c
> index 3cd52e1e70..f77da94bf6 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -82,8 +82,9 @@ void os_setup_signal_handling(void)
>
>  /*
>   * Find a likely location for support files using the location of the binary.
> + * Typically, this would be "$bindir/../share/qemu".
>   * When running from the build tree this will be "$bindir/../pc-bios".
> - * Otherwise, this is CONFIG_QEMU_DATADIR.
> + * Otherwise, this is CONFIG_QEMU_DATADIR as constructed by configure.
>   */
>  char *os_find_datadir(void)
>  {
> @@ -93,6 +94,12 @@ char *os_find_datadir(void)
>      exec_dir = qemu_get_exec_dir();
>      g_return_val_if_fail(exec_dir != NULL, NULL);
>
> +    dir = g_build_filename(exec_dir, "..", "share", "qemu", NULL);
> +    if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> +        return g_steal_pointer(&dir);
> +    }
> +    g_free(dir);  /* no autofree this time */
> +
>      dir = g_build_filename(exec_dir, "..", "pc-bios", NULL);
>      if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
>          return g_steal_pointer(&dir);
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-06-16  9:19 ` Peter Maydell
@ 2020-06-16 15:37   ` Slater, Joseph
  2020-07-15 19:37   ` Peter Maydell
  1 sibling, 0 replies; 13+ messages in thread
From: Slater, Joseph @ 2020-06-16 15:37 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Marc-André Lureau, Paolo Bonzini, QEMU Developers, MacLeod, Randy

Hi Peter,

Specifically, 5.0 breaks the way yocto runqemu starts qemu-system-*.  The installation path which is not known at qemu build time, is <arbitrary-prefix>/usr/{bin,share}.  In general, though, I think it's a good idea to look in "reasonable" locations relative to the executable, maybe with command line options taking precedence.

Joe

-----Original Message-----
From: Peter Maydell <peter.maydell@linaro.org> 
Sent: Tuesday, June 16, 2020 2:20 AM
To: Slater, Joseph <joe.slater@windriver.com>
Cc: QEMU Developers <qemu-devel@nongnu.org>; MacLeod, Randy <Randy.MacLeod@windriver.com>; Marc-André Lureau <marcandre.lureau@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH 1/1] os_find_datadir: search as in version 4.2

On Tue, 16 Jun 2020 at 00:00, Joe Slater <joe.slater@windriver.com> wrote:
>
> Always look for ../share/qemu then ../pc-bios when looking for datadir.

Could you provide some more context, please? Why is this change useful; presumably we broke some setup in 5.0, but what exactly ?

I'm guessing this might be a regression introduced by commit
6dd2dacedd83d12328 so I'm ccing the relevant people.

> Signed-off-by: Joe Slater <joe.slater@windriver.com>
> ---
>  os-posix.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/os-posix.c b/os-posix.c
> index 3cd52e1e70..f77da94bf6 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -82,8 +82,9 @@ void os_setup_signal_handling(void)
>
>  /*
>   * Find a likely location for support files using the location of the binary.
> + * Typically, this would be "$bindir/../share/qemu".
>   * When running from the build tree this will be "$bindir/../pc-bios".
> - * Otherwise, this is CONFIG_QEMU_DATADIR.
> + * Otherwise, this is CONFIG_QEMU_DATADIR as constructed by configure.
>   */
>  char *os_find_datadir(void)
>  {
> @@ -93,6 +94,12 @@ char *os_find_datadir(void)
>      exec_dir = qemu_get_exec_dir();
>      g_return_val_if_fail(exec_dir != NULL, NULL);
>
> +    dir = g_build_filename(exec_dir, "..", "share", "qemu", NULL);
> +    if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> +        return g_steal_pointer(&dir);
> +    }
> +    g_free(dir);  /* no autofree this time */
> +
>      dir = g_build_filename(exec_dir, "..", "pc-bios", NULL);
>      if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
>          return g_steal_pointer(&dir);
> --
> 2.17.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-06-16  9:19 ` Peter Maydell
  2020-06-16 15:37   ` Slater, Joseph
@ 2020-07-15 19:37   ` Peter Maydell
  2020-07-15 19:57     ` Marc-André Lureau
  1 sibling, 1 reply; 13+ messages in thread
From: Peter Maydell @ 2020-07-15 19:37 UTC (permalink / raw)
  To: Joe Slater
  Cc: Marc-André Lureau, Paolo Bonzini, QEMU Developers, MacLeod, Randy

On Tue, 16 Jun 2020 at 10:19, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 16 Jun 2020 at 00:00, Joe Slater <joe.slater@windriver.com> wrote:
> >
> > Always look for ../share/qemu then ../pc-bios when looking for datadir.
>
> Could you provide some more context, please? Why is this
> change useful; presumably we broke some setup in 5.0, but
> what exactly ?
>
> I'm guessing this might be a regression introduced by commit
> 6dd2dacedd83d12328 so I'm ccing the relevant people.

Marco, Paolo: ping? Another user has just asked me the status
of this as they also ran into this regression in what directories
we search...

thanks
-- PMM


> > Signed-off-by: Joe Slater <joe.slater@windriver.com>
> > ---
> >  os-posix.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/os-posix.c b/os-posix.c
> > index 3cd52e1e70..f77da94bf6 100644
> > --- a/os-posix.c
> > +++ b/os-posix.c
> > @@ -82,8 +82,9 @@ void os_setup_signal_handling(void)
> >
> >  /*
> >   * Find a likely location for support files using the location of the binary.
> > + * Typically, this would be "$bindir/../share/qemu".
> >   * When running from the build tree this will be "$bindir/../pc-bios".
> > - * Otherwise, this is CONFIG_QEMU_DATADIR.
> > + * Otherwise, this is CONFIG_QEMU_DATADIR as constructed by configure.
> >   */
> >  char *os_find_datadir(void)
> >  {
> > @@ -93,6 +94,12 @@ char *os_find_datadir(void)
> >      exec_dir = qemu_get_exec_dir();
> >      g_return_val_if_fail(exec_dir != NULL, NULL);
> >
> > +    dir = g_build_filename(exec_dir, "..", "share", "qemu", NULL);
> > +    if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> > +        return g_steal_pointer(&dir);
> > +    }
> > +    g_free(dir);  /* no autofree this time */
> > +
> >      dir = g_build_filename(exec_dir, "..", "pc-bios", NULL);
> >      if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> >          return g_steal_pointer(&dir);
> > --
> > 2.17.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-07-15 19:37   ` Peter Maydell
@ 2020-07-15 19:57     ` Marc-André Lureau
  2020-08-08  1:35       ` Brian Norris
  0 siblings, 1 reply; 13+ messages in thread
From: Marc-André Lureau @ 2020-07-15 19:57 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Paolo Bonzini, Joe Slater, QEMU Developers, MacLeod, Randy

Hi

On Wed, Jul 15, 2020 at 11:37 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Tue, 16 Jun 2020 at 10:19, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 16 Jun 2020 at 00:00, Joe Slater <joe.slater@windriver.com> wrote:
> > >
> > > Always look for ../share/qemu then ../pc-bios when looking for datadir.
> >
> > Could you provide some more context, please? Why is this
> > change useful; presumably we broke some setup in 5.0, but
> > what exactly ?
> >
> > I'm guessing this might be a regression introduced by commit
> > 6dd2dacedd83d12328 so I'm ccing the relevant people.
>
> Marco, Paolo: ping? Another user has just asked me the status
> of this as they also ran into this regression in what directories
> we search...

Thanks for the heads-up, I didn't see that bug/mail. Indeed, that
commit assumed that either we run from a build directory or from an
installed qemu. It seems this is hybrid approach, which I didn't know
we supported. I'll check it.

cheers

>
> thanks
> -- PMM
>
>
> > > Signed-off-by: Joe Slater <joe.slater@windriver.com>
> > > ---
> > >  os-posix.c | 9 ++++++++-
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/os-posix.c b/os-posix.c
> > > index 3cd52e1e70..f77da94bf6 100644
> > > --- a/os-posix.c
> > > +++ b/os-posix.c
> > > @@ -82,8 +82,9 @@ void os_setup_signal_handling(void)
> > >
> > >  /*
> > >   * Find a likely location for support files using the location of the binary.
> > > + * Typically, this would be "$bindir/../share/qemu".
> > >   * When running from the build tree this will be "$bindir/../pc-bios".
> > > - * Otherwise, this is CONFIG_QEMU_DATADIR.
> > > + * Otherwise, this is CONFIG_QEMU_DATADIR as constructed by configure.
> > >   */
> > >  char *os_find_datadir(void)
> > >  {
> > > @@ -93,6 +94,12 @@ char *os_find_datadir(void)
> > >      exec_dir = qemu_get_exec_dir();
> > >      g_return_val_if_fail(exec_dir != NULL, NULL);
> > >
> > > +    dir = g_build_filename(exec_dir, "..", "share", "qemu", NULL);
> > > +    if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> > > +        return g_steal_pointer(&dir);
> > > +    }
> > > +    g_free(dir);  /* no autofree this time */
> > > +
> > >      dir = g_build_filename(exec_dir, "..", "pc-bios", NULL);
> > >      if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> > >          return g_steal_pointer(&dir);
> > > --
> > > 2.17.1
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-06-15 22:58 [PATCH 1/1] os_find_datadir: search as in version 4.2 Joe Slater
  2020-06-16  9:19 ` Peter Maydell
@ 2020-07-16 14:12 ` Marc-André Lureau
  2020-08-10  7:33 ` Marc-André Lureau
  2 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2020-07-16 14:12 UTC (permalink / raw)
  To: Joe Slater; +Cc: QEMU, randy.macleod

[-- Attachment #1: Type: text/plain, Size: 1821 bytes --]

Hi

On Tue, Jun 16, 2020 at 2:59 AM Joe Slater <joe.slater@windriver.com> wrote:

> Always look for ../share/qemu then ../pc-bios when looking for datadir.
>
> Signed-off-by: Joe Slater <joe.slater@windriver.com>
>

Looks good to me, with:
Fixes: 6dd2dacedd83d12328 ("os-posix: simplify os_find_datadir")
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Alternatively, we may want to check the binary install location to decide
which path to return. I sent a proposal "[RFC PATCH] os-posix: fix
regression for install-less datadir location".

---
>  os-posix.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/os-posix.c b/os-posix.c
> index 3cd52e1e70..f77da94bf6 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -82,8 +82,9 @@ void os_setup_signal_handling(void)
>
>  /*
>   * Find a likely location for support files using the location of the
> binary.
> + * Typically, this would be "$bindir/../share/qemu".
>   * When running from the build tree this will be "$bindir/../pc-bios".
> - * Otherwise, this is CONFIG_QEMU_DATADIR.
> + * Otherwise, this is CONFIG_QEMU_DATADIR as constructed by configure.
>   */
>  char *os_find_datadir(void)
>  {
> @@ -93,6 +94,12 @@ char *os_find_datadir(void)
>      exec_dir = qemu_get_exec_dir();
>      g_return_val_if_fail(exec_dir != NULL, NULL);
>
> +    dir = g_build_filename(exec_dir, "..", "share", "qemu", NULL);
> +    if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> +        return g_steal_pointer(&dir);
> +    }
> +    g_free(dir);  /* no autofree this time */
> +
>      dir = g_build_filename(exec_dir, "..", "pc-bios", NULL);
>      if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
>          return g_steal_pointer(&dir);
> --
> 2.17.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2742 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-07-15 19:57     ` Marc-André Lureau
@ 2020-08-08  1:35       ` Brian Norris
  2020-08-08 15:34         ` Peter Maydell
  0 siblings, 1 reply; 13+ messages in thread
From: Brian Norris @ 2020-08-08  1:35 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Peter Maydell, Joe Slater, MacLeod, Randy, QEMU Developers,
	Paolo Bonzini

Hello!

On Wed, Jul 15, 2020 at 11:57:14PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jul 15, 2020 at 11:37 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > On Tue, 16 Jun 2020 at 10:19, Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > On Tue, 16 Jun 2020 at 00:00, Joe Slater <joe.slater@windriver.com> wrote:
> > > >
> > > > Always look for ../share/qemu then ../pc-bios when looking for datadir.
> > >
> > > Could you provide some more context, please? Why is this
> > > change useful; presumably we broke some setup in 5.0, but
> > > what exactly ?
> > >
> > > I'm guessing this might be a regression introduced by commit
> > > 6dd2dacedd83d12328 so I'm ccing the relevant people.
> >
> > Marco, Paolo: ping? Another user has just asked me the status
> > of this as they also ran into this regression in what directories
> > we search...
> 
> Thanks for the heads-up, I didn't see that bug/mail. Indeed, that
> commit assumed that either we run from a build directory or from an
> installed qemu. It seems this is hybrid approach, which I didn't know
> we supported. I'll check it.

Add one more to the pile! Chrome OS noticed this when upgrading to
5.0.0:

https://bugs.chromium.org/p/chromium/issues/detail?id=1114204#c8

I'd love to see this applied to a release.

I actually wrote basically this exact same patch and was about to submit
it, when I saw that this was already here. I've tested Joe's variant:

Tested-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>

The Chromium bug report is public, so you can see details there, but
it's basically the same -- we sometimes run qemu from a path that's not
the same absolute path noted in ./configure. This is because we build
qemu to run within our SDK (a semi-containerized chroot), but we also
support running that same QEMU binary from outside the container, which
then may be at some arbitrary hierarchy on a developer's machine.

It might be wise to include a tiny bit more verbose of a code comment,
to prevent oversights like this in the future. I'm sure that could be
spliced in when the patch is applied though.

Regards,
Brian


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-08-08  1:35       ` Brian Norris
@ 2020-08-08 15:34         ` Peter Maydell
  2020-08-10  7:29           ` Marc-André Lureau
  0 siblings, 1 reply; 13+ messages in thread
From: Peter Maydell @ 2020-08-08 15:34 UTC (permalink / raw)
  To: Brian Norris
  Cc: Marc-André Lureau, Joe Slater, MacLeod, Randy,
	QEMU Developers, Paolo Bonzini

On Sat, 8 Aug 2020 at 02:35, Brian Norris <briannorris@chromium.org> wrote:
>
> Hello!
>
> On Wed, Jul 15, 2020 at 11:57:14PM +0400, Marc-André Lureau wrote:
> > Hi
> >
> > On Wed, Jul 15, 2020 at 11:37 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > >
> > > On Tue, 16 Jun 2020 at 10:19, Peter Maydell <peter.maydell@linaro.org> wrote:
> > > >
> > > > On Tue, 16 Jun 2020 at 00:00, Joe Slater <joe.slater@windriver.com> wrote:
> > > > >
> > > > > Always look for ../share/qemu then ../pc-bios when looking for datadir.
> > > >
> > > > Could you provide some more context, please? Why is this
> > > > change useful; presumably we broke some setup in 5.0, but
> > > > what exactly ?
> > > >
> > > > I'm guessing this might be a regression introduced by commit
> > > > 6dd2dacedd83d12328 so I'm ccing the relevant people.
> > >
> > > Marco, Paolo: ping? Another user has just asked me the status
> > > of this as they also ran into this regression in what directories
> > > we search...
> >
> > Thanks for the heads-up, I didn't see that bug/mail. Indeed, that
> > commit assumed that either we run from a build directory or from an
> > installed qemu. It seems this is hybrid approach, which I didn't know
> > we supported. I'll check it.
>
> Add one more to the pile! Chrome OS noticed this when upgrading to
> 5.0.0:
>
> https://bugs.chromium.org/p/chromium/issues/detail?id=1114204#c8
>
> I'd love to see this applied to a release.

It's just missed 5.1, unfortunately :-(

Marc-André, did you want to review it ?

thanks
-- PMM


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-08-08 15:34         ` Peter Maydell
@ 2020-08-10  7:29           ` Marc-André Lureau
  2020-08-10 21:41             ` Brian Norris
  0 siblings, 1 reply; 13+ messages in thread
From: Marc-André Lureau @ 2020-08-10  7:29 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Paolo Bonzini, Joe Slater, Brian Norris, QEMU Developers, MacLeod, Randy

Hi

On Sat, Aug 8, 2020 at 7:34 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Sat, 8 Aug 2020 at 02:35, Brian Norris <briannorris@chromium.org> wrote:
> >
> > Hello!
> >
> > On Wed, Jul 15, 2020 at 11:57:14PM +0400, Marc-André Lureau wrote:
> > > Hi
> > >
> > > On Wed, Jul 15, 2020 at 11:37 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > > >
> > > > On Tue, 16 Jun 2020 at 10:19, Peter Maydell <peter.maydell@linaro.org> wrote:
> > > > >
> > > > > On Tue, 16 Jun 2020 at 00:00, Joe Slater <joe.slater@windriver.com> wrote:
> > > > > >
> > > > > > Always look for ../share/qemu then ../pc-bios when looking for datadir.
> > > > >
> > > > > Could you provide some more context, please? Why is this
> > > > > change useful; presumably we broke some setup in 5.0, but
> > > > > what exactly ?
> > > > >
> > > > > I'm guessing this might be a regression introduced by commit
> > > > > 6dd2dacedd83d12328 so I'm ccing the relevant people.
> > > >
> > > > Marco, Paolo: ping? Another user has just asked me the status
> > > > of this as they also ran into this regression in what directories
> > > > we search...
> > >
> > > Thanks for the heads-up, I didn't see that bug/mail. Indeed, that
> > > commit assumed that either we run from a build directory or from an
> > > installed qemu. It seems this is hybrid approach, which I didn't know
> > > we supported. I'll check it.
> >
> > Add one more to the pile! Chrome OS noticed this when upgrading to
> > 5.0.0:
> >
> > https://bugs.chromium.org/p/chromium/issues/detail?id=1114204#c8
> >
> > I'd love to see this applied to a release.
>
> It's just missed 5.1, unfortunately :-(
>
> Marc-André, did you want to review it ?

I tried an alternative approach, and ack his version instead:

https://patchew.org/QEMU/20200716141100.398296-1-marcandre.lureau@redhat.com/

(I am going to do this in this thread instead)



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-06-15 22:58 [PATCH 1/1] os_find_datadir: search as in version 4.2 Joe Slater
  2020-06-16  9:19 ` Peter Maydell
  2020-07-16 14:12 ` Marc-André Lureau
@ 2020-08-10  7:33 ` Marc-André Lureau
  2 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2020-08-10  7:33 UTC (permalink / raw)
  To: Joe Slater; +Cc: QEMU, randy.macleod

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]

On Tue, Jun 16, 2020 at 2:59 AM Joe Slater <joe.slater@windriver.com> wrote:

> Always look for ../share/qemu then ../pc-bios when looking for datadir.
>
> Signed-off-by: Joe Slater <joe.slater@windriver.com>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
>  os-posix.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/os-posix.c b/os-posix.c
> index 3cd52e1e70..f77da94bf6 100644
> --- a/os-posix.c
> +++ b/os-posix.c
> @@ -82,8 +82,9 @@ void os_setup_signal_handling(void)
>
>  /*
>   * Find a likely location for support files using the location of the
> binary.
> + * Typically, this would be "$bindir/../share/qemu".
>   * When running from the build tree this will be "$bindir/../pc-bios".
> - * Otherwise, this is CONFIG_QEMU_DATADIR.
> + * Otherwise, this is CONFIG_QEMU_DATADIR as constructed by configure.
>   */
>  char *os_find_datadir(void)
>  {
> @@ -93,6 +94,12 @@ char *os_find_datadir(void)
>      exec_dir = qemu_get_exec_dir();
>      g_return_val_if_fail(exec_dir != NULL, NULL);
>
> +    dir = g_build_filename(exec_dir, "..", "share", "qemu", NULL);
> +    if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
> +        return g_steal_pointer(&dir);
> +    }
> +    g_free(dir);  /* no autofree this time */
> +
>      dir = g_build_filename(exec_dir, "..", "pc-bios", NULL);
>      if (g_file_test(dir, G_FILE_TEST_IS_DIR)) {
>          return g_steal_pointer(&dir);
> --
> 2.17.1
>
>
>

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 2400 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-08-10  7:29           ` Marc-André Lureau
@ 2020-08-10 21:41             ` Brian Norris
  2021-01-23  2:05               ` Brian Norris
  0 siblings, 1 reply; 13+ messages in thread
From: Brian Norris @ 2020-08-10 21:41 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Peter Maydell, Joe Slater, MacLeod, Randy, QEMU Developers,
	Paolo Bonzini

On Mon, Aug 10, 2020 at 12:29 AM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
> On Sat, Aug 8, 2020 at 7:34 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > On Sat, 8 Aug 2020 at 02:35, Brian Norris <briannorris@chromium.org> wrote:
> > > Add one more to the pile! Chrome OS noticed this when upgrading to
> > > 5.0.0:
> > >
> > > https://bugs.chromium.org/p/chromium/issues/detail?id=1114204#c8
> > >
> > > I'd love to see this applied to a release.
> >
> > It's just missed 5.1, unfortunately :-(
> >
> > Marc-André, did you want to review it ?
>
> I tried an alternative approach, and ack his version instead:
>
> https://patchew.org/QEMU/20200716141100.398296-1-marcandre.lureau@redhat.com/
>
> (I am going to do this in this thread instead)

FWIW, you already provided your Review a month ago:
https://lore.kernel.org/qemu-devel/CAJ+F1CJmMV6pY6r0P6ukNZA_q+w6yLvAxekGnusgXYuuip6gZA@mail.gmail.com/

But I see you've now repeated it ;)
https://lore.kernel.org/qemu-devel/CAJ+F1CJdHo7R9rnmoD1cLEzsYLFsFVVjcAr5UcsYFgfcW3z2LA@mail.gmail.com/

In any case, thanks! We'll likely carry this patch in Chrome OS, until
it gets applied to a proper release.

Regards,
Brian


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2020-08-10 21:41             ` Brian Norris
@ 2021-01-23  2:05               ` Brian Norris
  2021-01-23 18:08                 ` Paolo Bonzini
  0 siblings, 1 reply; 13+ messages in thread
From: Brian Norris @ 2021-01-23  2:05 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Peter Maydell, Joe Slater, MacLeod, Randy, QEMU Developers,
	Paolo Bonzini

Just to follow-up here, since nobody followed up for months...

On Mon, Aug 10, 2020 at 2:41 PM Brian Norris <briannorris@chromium.org> wrote:
> On Mon, Aug 10, 2020 at 12:29 AM Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
> > On Sat, Aug 8, 2020 at 7:34 PM Peter Maydell <peter.maydell@linaro.org> wrote:
> > > On Sat, 8 Aug 2020 at 02:35, Brian Norris <briannorris@chromium.org> wrote:
> > > It's just missed 5.1, unfortunately :-(

And it missed 5.2 too :(

> > > Marc-André, did you want to review it ?
> >
> > I tried an alternative approach, and ack his version instead:
> >
> > https://patchew.org/QEMU/20200716141100.398296-1-marcandre.lureau@redhat.com/
> >
> > (I am going to do this in this thread instead)
>
> FWIW, you already provided your Review a month ago:
> https://lore.kernel.org/qemu-devel/CAJ+F1CJmMV6pY6r0P6ukNZA_q+w6yLvAxekGnusgXYuuip6gZA@mail.gmail.com/
>
> But I see you've now repeated it ;)
> https://lore.kernel.org/qemu-devel/CAJ+F1CJdHo7R9rnmoD1cLEzsYLFsFVVjcAr5UcsYFgfcW3z2LA@mail.gmail.com/
>
> In any case, thanks! We'll likely carry this patch in Chrome OS, until
> it gets applied to a proper release.

It turns out that Paolo inadvertently (?) fixed this issue by
refactoring, in v5.2.0:
ea1edcd7da1a vl: relocate paths to data directories
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=ea1edcd7da1a375ef7ccf8aa93b72827b518ad8e;hp=63c4db4c2e6d221cecb5aafa365934bb05724cb4

I tested that out here, and the new find_datadir() is able to track
relocations properly, by looking for a common directory ancestor of
the running executable. Thanks Paolo!

Brian


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/1] os_find_datadir: search as in version 4.2
  2021-01-23  2:05               ` Brian Norris
@ 2021-01-23 18:08                 ` Paolo Bonzini
  0 siblings, 0 replies; 13+ messages in thread
From: Paolo Bonzini @ 2021-01-23 18:08 UTC (permalink / raw)
  To: Brian Norris, Marc-André Lureau
  Cc: Peter Maydell, Joe Slater, QEMU Developers, MacLeod, Randy

On 23/01/21 03:05, Brian Norris wrote:
> It turns out that Paolo inadvertently (?) fixed this issue by
> refactoring, in v5.2.0:
> ea1edcd7da1a vl: relocate paths to data directories
> https://git.qemu.org/?p=qemu.git;a=commitdiff;h=ea1edcd7da1a375ef7ccf8aa93b72827b518ad8e;hp=63c4db4c2e6d221cecb5aafa365934bb05724cb4

That wasn't entirely inadvertent.  It had been on my todo list to fix 
relocatability for good, I just didn't remember how it got on the list. :)

So on one hand I didn't remember it to be a regression, and I only 
really set out to make QEMU relocatable in order to enable the change of 
commit d17f305a26 ("configure: use a platform-neutral prefix", 
2020-09-30).  On the other hand, your report was what had made me aware 
that relocatable installs were buggy and inconsistent.

Paolo



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-01-23 18:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-15 22:58 [PATCH 1/1] os_find_datadir: search as in version 4.2 Joe Slater
2020-06-16  9:19 ` Peter Maydell
2020-06-16 15:37   ` Slater, Joseph
2020-07-15 19:37   ` Peter Maydell
2020-07-15 19:57     ` Marc-André Lureau
2020-08-08  1:35       ` Brian Norris
2020-08-08 15:34         ` Peter Maydell
2020-08-10  7:29           ` Marc-André Lureau
2020-08-10 21:41             ` Brian Norris
2021-01-23  2:05               ` Brian Norris
2021-01-23 18:08                 ` Paolo Bonzini
2020-07-16 14:12 ` Marc-André Lureau
2020-08-10  7:33 ` Marc-André Lureau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).