Re: [PATCH 08/10] hw/9pfs: Allow using hw/9pfs with emscripten

[Christian Schoenebeck <qemu_oss@crudebyte.com>]

On Monday, April 7, 2025 4:45:59 PM CEST Kohei Tokunaga wrote:
> Emscripten's fiber does not support submitting coroutines to other
> threads. So this commit modifies hw/9pfs/coth.h to disable this behavior
> when compiled with Emscripten.

The lack of being able to dispatch a coroutine to a worker thread is one
thing, however it would probably still make sense to use fibers in 9pfs as
replacement of its coroutines mechanism.

In 9pfs coroutines are used to dispatch blocking fs I/O syscalls from main
thread to worker thread(s):

https://wiki.qemu.org/Documentation/9p#Control_Flow

If you just remove the coroutine code entirely, 9p server might hang for good,
and with it QEMU's main thread.

By using fibers instead, it would not hang, as it seems as if I/O syscalls are
emulated in Emscripten, right?

> Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
> ---
>  fsdev/file-op-9p.h     |  3 +++
>  fsdev/meson.build      |  2 +-
>  hw/9pfs/9p-util-stub.c | 43 ++++++++++++++++++++++++++++++++++++++++++
>  hw/9pfs/9p-util.h      | 18 ++++++++++++++++++
>  hw/9pfs/9p.c           |  3 +++
>  hw/9pfs/coth.h         | 12 ++++++++++++
>  hw/9pfs/meson.build    |  2 ++
>  meson.build            |  6 +++---
>  8 files changed, 85 insertions(+), 4 deletions(-)
>  create mode 100644 hw/9pfs/9p-util-stub.c
> 
> diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
> index 4997677460..b7ca2640ce 100644
> --- a/fsdev/file-op-9p.h
> +++ b/fsdev/file-op-9p.h
> @@ -26,6 +26,9 @@
>  # include <sys/param.h>
>  # include <sys/mount.h>
>  #endif
> +#ifdef EMSCRIPTEN
> +#include <sys/vfs.h>
> +#endif
>  
>  #define SM_LOCAL_MODE_BITS    0600
>  #define SM_LOCAL_DIR_MODE_BITS    0700
> diff --git a/fsdev/meson.build b/fsdev/meson.build
> index c751d8cb62..c3e92a29d7 100644
> --- a/fsdev/meson.build
> +++ b/fsdev/meson.build
> @@ -5,6 +5,6 @@ fsdev_ss.add(when: ['CONFIG_FSDEV_9P'], if_true: files(
>    '9p-marshal.c',
>    'qemu-fsdev.c',
>  ), if_false: files('qemu-fsdev-dummy.c'))
> -if host_os in ['linux', 'darwin']
> +if host_os in ['linux', 'darwin', 'emscripten']
>    system_ss.add_all(fsdev_ss)
>  endif
> diff --git a/hw/9pfs/9p-util-stub.c b/hw/9pfs/9p-util-stub.c
> new file mode 100644
> index 0000000000..57c89902ab
> --- /dev/null
> +++ b/hw/9pfs/9p-util-stub.c
> @@ -0,0 +1,43 @@
> +/*
> + * 9p utilities stub functions
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "9p-util.h"
> +
> +ssize_t fgetxattrat_nofollow(int dirfd, const char *path, const char *name,
> +                             void *value, size_t size)
> +{
> +    return -1;
> +}
> +
> +ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
> +                              char *list, size_t size)
> +{
> +    return -1;
> +}
> +
> +ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
> +                                const char *name)
> +{
> +    return -1;
> +}
> +
> +int fsetxattrat_nofollow(int dirfd, const char *path, const char *name,
> +                         void *value, size_t size, int flags)
> +{
> +    return -1;
> +
> +}
> +
> +int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
> +{
> +    return -1;
> +}
> +
> +ssize_t fgetxattr(int fd, const char *name, void *value, size_t size)
> +{
> +    return -1;
> +}

Missing

    errno = ENOTSUP;

> diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> index 7bc4ec8e85..8c5006fcdc 100644
> --- a/hw/9pfs/9p-util.h
> +++ b/hw/9pfs/9p-util.h
> @@ -84,6 +84,24 @@ static inline int errno_to_dotl(int err) {
>      } else if (err == EOPNOTSUPP) {
>          err = 95; /* ==EOPNOTSUPP on Linux */
>      }
> +#elif defined(EMSCRIPTEN)
> +    /*
> +     * FIXME: Only most important errnos translated here yet, this should be
> +     * extended to as many errnos being translated as possible in future.
> +     */
> +    if (err == ENAMETOOLONG) {
> +        err = 36; /* ==ENAMETOOLONG on Linux */
> +    } else if (err == ENOTEMPTY) {
> +        err = 39; /* ==ENOTEMPTY on Linux */
> +    } else if (err == ELOOP) {
> +        err = 40; /* ==ELOOP on Linux */
> +    } else if (err == ENODATA) {
> +        err = 61; /* ==ENODATA on Linux */
> +    } else if (err == ENOTSUP) {
> +        err = 95; /* ==EOPNOTSUPP on Linux */
> +    } else if (err == EOPNOTSUPP) {
> +        err = 95; /* ==EOPNOTSUPP on Linux */
> +    }

Looks like you just copied the macOS errno translation code. That probably
doesn't make sense.

/Christian

>  #else
>  #error Missing errno translation to Linux for this host system
>  #endif
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 7cad2bce62..4f45f0edd3 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -4013,6 +4013,9 @@ out_nofid:
>   * Linux guests.
>   */
>  #define P9_XATTR_SIZE_MAX 65536
> +#elif defined(EMSCRIPTEN)
> +/* No support for xattr */
> +#define P9_XATTR_SIZE_MAX 0
>  #else
>  #error Missing definition for P9_XATTR_SIZE_MAX for this host system
>  #endif
> diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
> index 2c54249b35..7b0d05ba1b 100644
> --- a/hw/9pfs/coth.h
> +++ b/hw/9pfs/coth.h
> @@ -19,6 +19,7 @@
>  #include "qemu/coroutine-core.h"
>  #include "9p.h"
>  
> +#ifndef EMSCRIPTEN
>  /*
>   * we want to use bottom half because we want to make sure the below
>   * sequence of events.
> @@ -57,6 +58,17 @@
>          /* re-enter back to qemu thread */                              \
>          qemu_coroutine_yield();                                         \
>      } while (0)
> +#else
> +/*
> + * FIXME: implement this on emscripten but emscripten's coroutine
> + * implementation (fiber) doesn't support submitting a coroutine to other
> + * threads.
> + */
> +#define v9fs_co_run_in_worker(code_block)                               \
> +    do {                                                                \
> +        code_block;                                                     \
> +    } while (0)
> +#endif
>  
>  void co_run_in_worker_bh(void *);
>  int coroutine_fn v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *);
> diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
> index d35d4f44ff..04f85fb9e9 100644
> --- a/hw/9pfs/meson.build
> +++ b/hw/9pfs/meson.build
> @@ -17,6 +17,8 @@ if host_os == 'darwin'
>    fs_ss.add(files('9p-util-darwin.c'))
>  elif host_os == 'linux'
>    fs_ss.add(files('9p-util-linux.c'))
> +elif host_os == 'emscripten'
> +  fs_ss.add(files('9p-util-stub.c'))
>  endif
>  fs_ss.add(when: 'CONFIG_XEN_BUS', if_true: files('xen-9p-backend.c'))
>  system_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
> diff --git a/meson.build b/meson.build
> index ab84820bc5..a3aadf8b59 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -2356,11 +2356,11 @@ dbus_display = get_option('dbus_display') \
>    .allowed()
>  
>  have_virtfs = get_option('virtfs') \
> -    .require(host_os == 'linux' or host_os == 'darwin',
> +    .require(host_os == 'linux' or host_os == 'darwin' or host_os == 'emscripten',
>               error_message: 'virtio-9p (virtfs) requires Linux or macOS') \
> -    .require(host_os == 'linux' or cc.has_function('pthread_fchdir_np'),
> +    .require(host_os == 'linux' or host_os == 'emscripten' or cc.has_function('pthread_fchdir_np'),
>               error_message: 'virtio-9p (virtfs) on macOS requires the presence of pthread_fchdir_np') \
> -    .require(host_os == 'darwin' or libattr.found(),
> +    .require(host_os == 'darwin' or host_os == 'emscripten' or libattr.found(),
>               error_message: 'virtio-9p (virtfs) on Linux requires libattr-devel') \
>      .disable_auto_if(not have_tools and not have_system) \
>      .allowed()
>