| Message ID | 20171028194833.23858-1-n54@gmx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 >
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
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 >
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';
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(-)