[Qemu-devel] [PATCH] oslib-posix: Use sysctl(2) call to resolve exec_dir on NetBSD

[Qemu-devel] [PATCH] oslib-posix: Use sysctl(2) call to resolve exec_dir on NetBSD

[Kamil Rytarowski]

NetBSD 8.0(beta) ships with KERN_PROC_PATHNAME in sysctl(2).
Older NetBSD versions can use argv[0] parsing fallback.

This code section is partly shared with FreeBSD.

Signed-off-by: Kamil Rytarowski <n54@gmx.com>
---
 util/oslib-posix.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 382bd4a231..77369c92ce 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -49,6 +49,10 @@
 #include <libutil.h>
 #endif
 
+#ifdef __NetBSD__
+#include <sys/sysctl.h>
+#endif
+
 #include "qemu/mmap-alloc.h"
 
 #ifdef CONFIG_DEBUG_STACK_USAGE
@@ -250,9 +254,14 @@ void qemu_init_exec_dir(const char *argv0)
             p = buf;
         }
     }
-#elif defined(__FreeBSD__)
+#elif defined(__FreeBSD__) \
+      || (defined(__NetBSD__) && defined(KERN_PROC_PATHNAME))
     {
+#if defined(__FreeBSD__)
         static int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
+#else
+        static int mib[4] = {CTL_KERN, KERN_PROC_ARGS, -1, KERN_PROC_PATHNAME};
+#endif
         size_t len = sizeof(buf) - 1;
 
         *buf = '\0';
-- 
2.14.3

Re: [Qemu-devel] [PATCH] oslib-posix: Use sysctl(2) call to resolve exec_dir on NetBSD

[Peter Maydell]

On 28 October 2017 at 20:48, Kamil Rytarowski <n54@gmx.com> wrote:
> NetBSD 8.0(beta) ships with KERN_PROC_PATHNAME in sysctl(2).
> Older NetBSD versions can use argv[0] parsing fallback.
>
> This code section is partly shared with FreeBSD.
>
> Signed-off-by: Kamil Rytarowski <n54@gmx.com>
> ---
>  util/oslib-posix.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 382bd4a231..77369c92ce 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -49,6 +49,10 @@
>  #include <libutil.h>
>  #endif
>
> +#ifdef __NetBSD__
> +#include <sys/sysctl.h>
> +#endif
> +
>  #include "qemu/mmap-alloc.h"
>
>  #ifdef CONFIG_DEBUG_STACK_USAGE
> @@ -250,9 +254,14 @@ void qemu_init_exec_dir(const char *argv0)
>              p = buf;
>          }
>      }
> -#elif defined(__FreeBSD__)
> +#elif defined(__FreeBSD__) \
> +      || (defined(__NetBSD__) && defined(KERN_PROC_PATHNAME))
>      {
> +#if defined(__FreeBSD__)
>          static int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
> +#else
> +        static int mib[4] = {CTL_KERN, KERN_PROC_ARGS, -1, KERN_PROC_PATHNAME};
> +#endif
>          size_t len = sizeof(buf) - 1;
>
>          *buf = '\0';

It's a shame the BSDs can't agree on a single way to implement this :-(

I had a look at how Go implements this:
https://github.com/golang/go/commit/2fc67e71af142bfa1e7662a4fde361f43509d2d7

and for NetBSD it uses readlink("/proc/curproc/exe"), which would be
a better fallback for "sysctl not implemented" than the argv[0]
stuff, perhaps (but then nobody's complained much so perhaps not
worth the effort now). It also has /proc/curproc/file for OpenBSD, but
my OpenBSD VM doesn't mount /proc/, which reduces my enthusiasm
for trying for a "generic for BSDs try various /proc/ paths" approach.

In any case, I think this is better than what we have today,
so I've applied it to master.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] oslib-posix: Use sysctl(2) call to resolve exec_dir on NetBSD

[Kamil Rytarowski]

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

On 02.11.2017 17:55, Peter Maydell wrote:
> On 28 October 2017 at 20:48, Kamil Rytarowski <n54@gmx.com> wrote:
>> NetBSD 8.0(beta) ships with KERN_PROC_PATHNAME in sysctl(2).
>> Older NetBSD versions can use argv[0] parsing fallback.
>>
>> This code section is partly shared with FreeBSD.
>>
>> Signed-off-by: Kamil Rytarowski <n54@gmx.com>
>> ---
>>  util/oslib-posix.c | 11 ++++++++++-
>>  1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>> index 382bd4a231..77369c92ce 100644
>> --- a/util/oslib-posix.c
>> +++ b/util/oslib-posix.c
>> @@ -49,6 +49,10 @@
>>  #include <libutil.h>
>>  #endif
>>
>> +#ifdef __NetBSD__
>> +#include <sys/sysctl.h>
>> +#endif
>> +
>>  #include "qemu/mmap-alloc.h"
>>
>>  #ifdef CONFIG_DEBUG_STACK_USAGE
>> @@ -250,9 +254,14 @@ void qemu_init_exec_dir(const char *argv0)
>>              p = buf;
>>          }
>>      }
>> -#elif defined(__FreeBSD__)
>> +#elif defined(__FreeBSD__) \
>> +      || (defined(__NetBSD__) && defined(KERN_PROC_PATHNAME))
>>      {
>> +#if defined(__FreeBSD__)
>>          static int mib[4] = {CTL_KERN, KERN_PROC, KERN_PROC_PATHNAME, -1};
>> +#else
>> +        static int mib[4] = {CTL_KERN, KERN_PROC_ARGS, -1, KERN_PROC_PATHNAME};
>> +#endif
>>          size_t len = sizeof(buf) - 1;
>>
>>          *buf = '\0';
> 
> It's a shame the BSDs can't agree on a single way to implement this :-(
> 
> I had a look at how Go implements this:
> https://github.com/golang/go/commit/2fc67e71af142bfa1e7662a4fde361f43509d2d7
> 
> and for NetBSD it uses readlink("/proc/curproc/exe"), which would be
> a better fallback for "sysctl not implemented" than the argv[0]
> stuff, perhaps (but then nobody's complained much so perhaps not
> worth the effort now). It also has /proc/curproc/file for OpenBSD, but
> my OpenBSD VM doesn't mount /proc/, which reduces my enthusiasm
> for trying for a "generic for BSDs try various /proc/ paths" approach.
> 
> In any case, I think this is better than what we have today,
> so I've applied it to master.
> 

It's better to assume that there is no such entity as a shared BSD code
in modern BSD Operating Systems. My opinion is to drop CONFIG_BSD
entirely from qemu and treat __NetBSD__, __FreeBSD__ and __OpenBSD__ as
diverse targets.

(I'm not sure how about bsd-user, whether it should be dropped and
reimplemented on per-OS basis? I think that in the current state of
affairs it's a reasonable move forwards.).

OpenBSD does not ship any interface except argv[0] to get executable
path. They have removed their procfs.

NetBSD and FreeBSD deprecate procfs and fallback to /proc shall not be
introduced into new code.

> thanks
> -- PMM
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH] oslib-posix: Use sysctl(2) call to resolve exec_dir on NetBSD

[Peter Maydell]

On 2 November 2017 at 17:27, Kamil Rytarowski <n54@gmx.com> wrote:
> On 02.11.2017 17:55, Peter Maydell wrote:
>> It's a shame the BSDs can't agree on a single way to implement this :-(

> It's better to assume that there is no such entity as a shared BSD code
> in modern BSD Operating Systems. My opinion is to drop CONFIG_BSD
> entirely from qemu and treat __NetBSD__, __FreeBSD__ and __OpenBSD__ as
> diverse targets.

Mmm, that's probably the best way to do it. (Better still,
wherever possible check features rather than OS type).

It does mean that more minor BSD variants like DragonFly are
in a worse place, because nobody much is going to care about
supporting them, and they can't get automatic support by
being "like all of the others".

thanks
-- PMM

Re: [Qemu-devel] [PATCH] oslib-posix: Use sysctl(2) call to resolve exec_dir on NetBSD

[Kamil Rytarowski]

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

On 02.11.2017 18:29, Peter Maydell wrote:
> On 2 November 2017 at 17:27, Kamil Rytarowski <n54@gmx.com> wrote:
>> On 02.11.2017 17:55, Peter Maydell wrote:
>>> It's a shame the BSDs can't agree on a single way to implement this :-(
> 
>> It's better to assume that there is no such entity as a shared BSD code
>> in modern BSD Operating Systems. My opinion is to drop CONFIG_BSD
>> entirely from qemu and treat __NetBSD__, __FreeBSD__ and __OpenBSD__ as
>> diverse targets.
> 
> Mmm, that's probably the best way to do it. (Better still,
> wherever possible check features rather than OS type).
> 

Correct.

We already end up with check for CONFIG_BSD and specific BSD variation.

chardev/char-parallel.c:#ifdef CONFIG_BSD
chardev/char-parallel.c-#if defined(__FreeBSD__) ||
defined(__FreeBSD_kernel__)

util/qemu-openpty.c:#elif defined CONFIG_BSD
util/qemu-openpty.c-# include <termios.h>
util/qemu-openpty.c-# if defined(__FreeBSD__) ||
defined(__FreeBSD_kernel__) || defined(__DragonFly__)

block.c:#ifdef CONFIG_BSD
block.c-#include <sys/ioctl.h>
block.c-#include <sys/queue.h>
block.c-#ifndef __DragonFly__

Sometimes automatic detection whether a struct or function exists can
lead to problems as they might have different purpose. This is common
especially for sysctl(2) calls.

> It does mean that more minor BSD variants like DragonFly are
> in a worse place, because nobody much is going to care about
> supporting them, and they can't get automatic support by
> being "like all of the others".
> 

Fixing and maintaining DragonFly requires non-trivial (at lest in terms
of dedicated time) effort. I wouldn't be surprised that part of qemu
problems originates from the base-system bugs (this happens in every BSD
and OS).

CC: Antonio who expressed interest in the DFLY port.

> thanks
> -- PMM
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]