[v18,00/10] Shared library module support

> Looks like we're not getting the executable path
> correctly on MacOS for some reason.

Indeed, there is no code for it.  Can you try the patch below?

> Also, mjt pointed out on IRC that we probably want to allow
> installing binary modules into a path with the arch name in
> it, to allow for multiarch distros.

Isn't --libdir enough?

Paolo

On 6 February 2014 23:38, Paolo Bonzini <pbonzini@redhat.com> wrote:
>> Looks like we're not getting the executable path
>> correctly on MacOS for some reason.
>
> Indeed, there is no code for it.  Can you try the patch below?
>
>> Also, mjt pointed out on IRC that we probably want to allow
>> installing binary modules into a path with the arch name in
>> it, to allow for multiarch distros.
>
> Isn't --libdir enough?
>
> Paolo
>
> ==================== 8< ========================
> From: Paolo Bonzini <pbonzini@redhat.com>
> Date: Wed, 5 Feb 2014 21:15:01 +0100
> Subject: [PATCH] oslib: port qemu_exec_dir to Darwin
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  util/oslib-posix.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 372b2f9..590a7d1 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -59,6 +59,12 @@ extern int daemon(int, int);
>  #include <sys/mman.h>
>  #include <libgen.h>
>
> +/* Get declaration of _NSGetExecutablePath on MacOS X 10.2 or newer.  */
> +#if defined(__APPLE__) && defined(__MACH__)
> +#define ENUM_DYLD_BOOL
> +#include <mach-o/dyld.h>
> +#endif
> +
>  #ifdef CONFIG_LINUX
>  #include <sys/syscall.h>
>  #endif
> @@ -303,6 +309,15 @@ char *qemu_exec_dir(const char *argv0)
>              p = buf;
>          }
>      }
> +#elif defined(__APPLE__) && defined(__MACH__)
> +    {
> +        char result[PATH_MAX];
> +        uint32_t length = PATH_MAX;
> +        if (_NSGetExecutablePath (result, &length) != 0 || result[0] != '/') {
> +            return NULL;
> +        }
> +        p = realpath(result, buf);
> +    }
>  #endif
>      /* If we don't have any way of figuring out the actual executable
>         location then try argv[0].  */
> --
> 1.8.5.3

Why do we need OS specific code to do this when the code
in vl.c for getting the executable path to use as a base for
finding bios blobs works fine without OS specific code?

thanks
-- PMM

Il 07/02/2014 01:06, Peter Maydell ha scritto:
> Why do we need OS specific code to do this when the code
> in vl.c for getting the executable path to use as a base for
> finding bios blobs works fine without OS specific code?

That's because right now module_call_init is not receiving an argv[0] to 
pass to qemu_exec_dir (os_find_datadir has an argument for that).  It 
can be fixed (in the whole call chain, including e.g. 
bdrv_init/bdrv_init_with_whitelist).

However, note that there is OS-specific code already for Linux, FreeBSD 
and Windows.  Adding Darwin makes sense.

On some BSDs there's also /proc/curproc/file.  I don't know _which_ BSDs 
have it though.

Paolo

07.02.2014 11:13, Paolo Bonzini wrote:
> Il 07/02/2014 01:06, Peter Maydell ha scritto:
>> Why do we need OS specific code to do this when the code
>> in vl.c for getting the executable path to use as a base for
>> finding bios blobs works fine without OS specific code?
> 
> That's because right now module_call_init is not receiving an argv[0] to pass to qemu_exec_dir (os_find_datadir has an argument for that).  It can be fixed (in the whole call chain, including e.g. bdrv_init/bdrv_init_with_whitelist).

We can add a global variable (either executable_dir or whole -L path),
init it in vl.c as we do now, and use it in other places in a uniform
way.

BTW, It'd be really good to have a separate, arch-specific directory for
the plugins, like /usr/lib/x86_64-linux-gnu/qemu/, so it will be
multiarch-able.  But it is not really a show-stopper.

Thanks,

/mjt

Il 07/02/2014 09:14, Michael Tokarev ha scritto:
> 07.02.2014 11:13, Paolo Bonzini wrote:
>> > Il 07/02/2014 01:06, Peter Maydell ha scritto:
>>> >> Why do we need OS specific code to do this when the code
>>> >> in vl.c for getting the executable path to use as a base for
>>> >> finding bios blobs works fine without OS specific code?
>> >
>> > That's because right now module_call_init is not receiving an argv[0] to pass to qemu_exec_dir (os_find_datadir has an argument for that).  It can be fixed (in the whole call chain, including e.g. bdrv_init/bdrv_init_with_whitelist).
>
> We can add a global variable (either executable_dir or whole -L path),
> init it in vl.c as we do now, and use it in other places in a uniform
> way.

FWIW there's not just vl.c, there's also qemu-io.c and qemu-img.c.  In 
the future tests could also need to do the same.

> BTW, It'd be really good to have a separate, arch-specific directory for
> the plugins, like /usr/lib/x86_64-linux-gnu/qemu/, so it will be
> multiarch-able.  But it is not really a show-stopper.

Why isn't it enough to do --libdir=/usr/lib/x86_64-linux-gnu?

Paolo

On Fri, 02/07 12:14, Michael Tokarev wrote:
> 07.02.2014 11:13, Paolo Bonzini wrote:
> > Il 07/02/2014 01:06, Peter Maydell ha scritto:
> >> Why do we need OS specific code to do this when the code
> >> in vl.c for getting the executable path to use as a base for
> >> finding bios blobs works fine without OS specific code?
> > 
> > That's because right now module_call_init is not receiving an argv[0] to pass to qemu_exec_dir (os_find_datadir has an argument for that).  It can be fixed (in the whole call chain, including e.g. bdrv_init/bdrv_init_with_whitelist).
> 
> We can add a global variable (either executable_dir or whole -L path),
> init it in vl.c as we do now, and use it in other places in a uniform
> way.

"-L" path doesn't apply to command line tools such as qemu-img, but yes,
storing the executable_dir will be helpful anyway.

> 
> BTW, It'd be really good to have a separate, arch-specific directory for
> the plugins, like /usr/lib/x86_64-linux-gnu/qemu/, so it will be
> multiarch-able.  But it is not really a show-stopper.
> 

An optional --moddir configure option would do, but it can be a separate patch.

Fam