[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201029031121.GJ534@brightrain.aerifal.cx>
Date: Wed, 28 Oct 2020 23:11:22 -0400
From: Rich Felker <dalias@...c.org>
To: musl@...ts.openwall.com
Subject: Re: qemu-user and musl
On Wed, Oct 28, 2020 at 04:50:29PM -0700, Michael Forney wrote:
> Hi,
>
> I'm trying to get various musl compatibility issues in qemu fixed
> upstream. Here are the issues I've encountered:
Thanks! This is a long-wanted goal!
> 1. To implement the timer_create syscall, qemu just translates
> between host and target sigevent, then calls the libc timer_create.
> For SIGEV_THREAD, both glibc and musl implement this using the
> Linux-specific SIGEV_THREAD_ID. However, musl's timer_create does
> not support SIGEV_THREAD_ID from the application, so it fails with
> EINVAL. This means that any application using timers with SIGEV_THREAD
> does not work with qemu-user running on musl.
>
> This issue appears as a build time error due to the use of
> glibc-internal _sigev_un._tid member during sigevent translation.
> Most distributions patch this by introducing a `host_sigevent`
> matching the glibc layout and translating to that instead of libc's
> sigevent. For the reasons mentioned above, this does not actually
> fix the problem.
>
> I see there is a patch on the mailing list adding support for
> SIGEV_THREAD_ID which looks ready to merge. Rich, since you're
> working towards getting ready for the next musl release, do you
> think this might make it in? That way, we could just patch qemu to
> add
>
> #ifndef sigev_notify_thread_id
> #define sigev_notify_thread_id _sigev_un._tid
> #endif
>
> and replace _sigev_un._tid with sigev_notify_thread_id.
I think that's okay, and I'm merging the patch now (thanks for
reminding me!), but I'm not sure if it will work still. The signal
number used as SIGTIMER is considered an implementation-reserved
internal signal by musl, and as such it can't be poked at by any of
the public signal API -- and even if it could be, poking at it would
break things.
Future musl will probably drop this signal entirley and implement
SIGEV_THREAD timers in userspace with a clock_nanosleep loop. It would
be a lot cleaner, but my only reservation is about whether it can be
made to work with Linux's wake-on-timer functionality that users might
want.
> 2. qemu uses long obsolete F_SHLCK and F_EXLCK in the translation
> of struct flock between host and target, which musl does not define.
> Most musl distributions patch qemu to define these constants itself
> if they are missing, but seeing as these lock types are unsupported
> by Linux since 2.2, I sent a patch to just drop them (you'll get
> EINVAL either way).
These are hideous legacy artifacts of ancient Linux's attempt to let
libc emulate BSD flock() on top of fcntl, which did not work. qemu
should just drop them. They're not working.
> 3. qemu uses the following configure test to check for clock_adjtime:
>
> #include <time.h>
>
> int main(void)
> {
> return clock_adjtime(0, 0);
> }
>
> However, musl declares clock_adjtime in sys/timex.h, as indicated
> by the linux man page, while glibc only declares it in time.h (i.e.
> it is not just a case of glibc implicitly including other headers).
> So, including one or the other is not enough. A real application
> will need both anyway for the clockid_t values and struct timex,
> but this mismatch seems a bit strange and I'm not sure if it was
> deliberate.
>
> (of course, this is easy to fix by just adding an include of
> <sys/timex.h> to the test, but I thought I'd mention it in case
> there was anything actionable here)
I think it should be left alone. clock_adjtime is not usable without
sys/timex.h anyway, and struct timex can't be exposed in time.h
(although I guess it could be exposed as an incomplete type only with
_GNU_SOURCE, but that might not even help). Applications should just
follow the documentation and use sys/timex.h.
> 4. Until recently qemu used the __SIGRTMIN and __SIGRTMAX macros
> in its signal translation code. It looks like this was recently
> refactored, and all that remains is a static assert:
>
> QEMU_BUILD_BUG_ON(__SIGRTMAX + 1 != _NSIG);
>
> I think the intention is to ensure that any possible value of
> SIGRTMAX is smaller than _NSIG and will not index past past the end
> of the host-to-target translation table. However, I think this is
> a safe assumption and the assert can be dropped (or at least, wrapped
> in #ifdef __SIGRTMAX).
That sounds correct to me.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
