Looks nice.

I see you added a general technique whereby every .so in /usr/lib/fs is
loaded automatically during boot (not only when mount() is actually called).
Make a lot of sense. We can even make it more general, e.g., /lib/modules
(or any other name), indicating in general features loaded at boot-time,
not just filesystems.

--
Nadav Har'El
n...@scylladb.com


On Fri, Mar 27, 2020 at 6:35 PM Commit Bot <b...@cloudius-systems.com> wrote:

> From: Waldemar Kozaczuk <jwkozac...@gmail.com>
> Committer: Waldemar Kozaczuk <jwkozac...@gmail.com>
> Branch: master
>
> fs: move nfs support out of kernel in lieu of a separate pluggable module
>
> This patch removes external/fs/libnfs module and makes nfs support
> pluggable
> by moving it into a module (shared library) that can be added to the image
> instead of being compiled into the kernel using nfs=true build option.
> The nfs support can be added by adding nfs module to the image.
>
> More specifically:
> - external/fs/libnfs is removed and equivalent modules/libnfs gets created
> - most fs/nfs code except for fs_null_vfsops.cc gets moved out of a kernel
>   into new modules/nfs that is built as a shared library
> - vfs mount logic is able to dynamically load extra filesystem
>   libraries from /usr/lib/fs
>
> Completes #1078
>
> Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
>
> ---
> diff --git a/.gitmodules b/.gitmodules
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -15,6 +15,3 @@
>  [submodule "modules/httpserver/osv-gui"]
>         path = modules/httpserver-html5-gui/osv-gui
>         url = ../../cloudius-systems/osv-gui.git
> -[submodule "external/fs/libnfs"]
> -       path = external/fs/libnfs
> -       url = https://github.com/sahlberg/libnfs.git
> diff --git a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -4,8 +4,6 @@
>  # This work is open source software, licensed under the terms of the
>  # BSD license as described in the LICENSE file in the top-level directory.
>
> -# The nfs=true flag will build in the NFS client filesystem support
> -
>  # Delete the builtin make rules, as if "make -r" was used.
>  .SUFFIXES:
>
> @@ -142,25 +140,10 @@ check:
>         ./scripts/build check
>  .PHONY: check
>
> -libnfs-path = external/fs/libnfs/
> -
> -$(out)/libnfs.a:
> -       cd $(libnfs-path) && \
> -       $(call quiet, ./bootstrap) && \
> -       $(call quiet, ./configure --enable-shared=no --enable-static=yes
> --enable-silent-rules) && \
> -       $(call quiet, make)
> -       $(call quiet, cp -a $(libnfs-path)/lib/.libs/libnfs.a
> $(out)/libnfs.a)
> -
> -clean-libnfs:
> -       if [ -f $(out)/libnfs.a ] ; then \
> -       cd $(libnfs-path) && \
> -       make distclean; \
> -       fi
> -
>  # Remember that "make clean" needs the same parameters that set $(out) in
>  # the first place, so to clean the output of "make mode=debug" you need to
>  # do "make mode=debug clean".
> -clean: clean-libnfs
> +clean:
>         rm -rf $(out)
>         rm -f $(outlink) $(outlink2)
>  .PHONY: clean
> @@ -374,7 +357,7 @@ tools += tools/uush/uush.so
>  tools += tools/uush/ls.so
>  tools += tools/uush/mkdir.so
>
> -tools += tools/mount/mount-nfs.so
> +tools += tools/mount/mount-fs.so
>  tools += tools/mount/umount.so
>
>  ifeq ($(arch),aarch64)
> @@ -1849,14 +1832,7 @@ endif
>
>  boost-libs := $(boost-lib-dir)/libboost_system$(boost-mt).a
>
> -ifeq ($(nfs), true)
> -       nfs-lib = $(out)/libnfs.a
> -       nfs_o = nfs.o nfs_vfsops.o nfs_vnops.o
> -else
> -       nfs_o = nfs_null_vfsops.o
> -endif
> -
> -objects += $(addprefix fs/nfs/, $(nfs_o))
> +objects += fs/nfs/nfs_null_vfsops.o
>
>  # ld has a known bug (
> https://sourceware.org/bugzilla/show_bug.cgi?id=6468)
>  # where if the executable doesn't use shared libraries, its .dynamic
> section
> @@ -1865,7 +1841,7 @@ objects += $(addprefix fs/nfs/, $(nfs_o))
>  $(out)/dummy-shlib.so: $(out)/dummy-shlib.o
>         $(call quiet, $(CXX) -nodefaultlibs -shared $(gcc-sysroot) -o $@
> $^, LINK $@)
>
> -stage1_targets = $(out)/arch/$(arch)/boot.o $(out)/loader.o
> $(out)/runtime.o $(drivers:%=$(out)/%) $(objects:%=$(out)/%)
> $(out)/dummy-shlib.so $(nfs-lib)
> +stage1_targets = $(out)/arch/$(arch)/boot.o $(out)/loader.o
> $(out)/runtime.o $(drivers:%=$(out)/%) $(objects:%=$(out)/%)
> $(out)/dummy-shlib.so
>  stage1: $(stage1_targets) links
>  .PHONY: stage1
>
> diff --git a/external/fs/libnfs b/external/fs/libnfs
> --- a/external/fs/libnfs
> +++ b/external/fs/libnfs
> @@ -1 +0,0 @@
> -Subproject commit dc8d86628d2f67cb4b7a9e5e5d7a1259f065b3c7
> diff --git a/fs/nfs/nfs_null_vfsops.cc b/fs/nfs/nfs_null_vfsops.cc
> --- a/fs/nfs/nfs_null_vfsops.cc
> +++ b/fs/nfs/nfs_null_vfsops.cc
> @@ -15,15 +15,22 @@
>  #define nfs_vget    ((vfsop_vget_t)vfs_nullop)
>  #define nfs_statfs  ((vfsop_statfs_t)vfs_nullop)
>
> +static int nfs_noop_mount(struct mount *mp, const char *dev, int flags,
> +                          const void *data)
> +{
> +    printf("The nfs module is in-active!. Please add nfs module to the
> image.\n");
> +    return -1;
> +}
> +
>  /*
>   * File system operations
>   *
>   * This desactivate the NFS file system when libnfs is not compiled in.
>   *
>   */
>  struct vfsops nfs_vfsops = {
> -    nfs_mount,      /* mount */
> -    nfs_umount,    /* umount */
> +    nfs_noop_mount,      /* mount */
> +    nfs_umount,     /* umount */
>      nfs_sync,       /* sync */
>      nfs_vget,       /* vget */
>      nfs_statfs,     /* statfs */
> diff --git a/fs/vfs/main.cc b/fs/vfs/main.cc
> --- a/fs/vfs/main.cc
> +++ b/fs/vfs/main.cc
> @@ -53,6 +53,7 @@
>  #include <fcntl.h>
>  #undef open
>  #undef fcntl
> +#include <dlfcn.h>
>
>  #include <osv/prex.h>
>  #include <osv/vnode.h>
> @@ -2305,6 +2306,22 @@ void pivot_rootfs(const char* path)
>      if (ret)
>          kprintf("failed to pivot root, error = %s\n", strerror(ret));
>
> +    // Initialize other filesystem libraries if present
> +    auto fs_lib_dir = opendir("/usr/lib/fs");
> +    if (fs_lib_dir) {
> +        while (auto dirent = readdir(fs_lib_dir)) {
> +            auto len = strlen(dirent->d_name);
> +            if (len >= 3 && strcmp(dirent->d_name + (len - 3), ".so") ==
> 0) {
> +                auto lib_path = std::string("/usr/lib/fs/") +
> dirent->d_name;
> +                auto module = dlopen(lib_path.c_str(), RTLD_LAZY);
> +                if (module)
> +                    debugf("VFS: Initialized filesystem library: %s\n",
> lib_path.c_str());
> +            }
> +        }
> +
> +        closedir(fs_lib_dir);
> +    }
> +
>      auto ent = setmntent("/etc/fstab", "r");
>      if (!ent) {
>          return;
> diff --git a/fs/vfs/vfs_conf.cc b/fs/vfs/vfs_conf.cc
> --- a/fs/vfs/vfs_conf.cc
> +++ b/fs/vfs/vfs_conf.cc
> @@ -71,7 +71,7 @@ const struct vfssw vfssw[] = {
>         {"devfs",       devfs_init,     &devfs_vfsops},
>         {"nfs",         nfs_init,       &nfs_vfsops},
>         {"procfs",      procfs_init,    &procfs_vfsops},
> -    {"sysfs",  sysfs_init,     &sysfs_vfsops},
> +       {"sysfs",       sysfs_init,     &sysfs_vfsops},
>         {"zfs",         zfs_init,       &zfs_vfsops},
>         {"rofs",        rofs_init,      &rofs_vfsops},
>         {"virtiofs",    virtiofs_init,  &virtiofs_vfsops},
> diff --git a/modules/libnfs/.gitignore b/modules/libnfs/.gitignore
> --- a/modules/libnfs/.gitignore
> +++ b/modules/libnfs/.gitignore
> @@ -0,0 +1 @@
> +upstream
> diff --git a/modules/libnfs/Makefile b/modules/libnfs/Makefile
> --- a/modules/libnfs/Makefile
> +++ b/modules/libnfs/Makefile
> @@ -0,0 +1,19 @@
> +src = $(shell readlink -f ../..)
> +module-dir = $(src)/modules/libnfs
> +
> +all: module
> +module: libnfs
> +
> +libnfs: upstream/libnfs/.git upstream/libnfs/lib/libnfs.so.4.0.0
> +
> +.PHONY: libnfs
> +
> +upstream/libnfs/.git:
> +       mkdir -p $(module-dir)/upstream && cd $(module-dir)/upstream && \
> +       git clone --depth 1 https://github.com/sahlberg/libnfs.git
> +
> +upstream/libnfs/lib/libnfs.so.4.0.0:
> +       cd $(module-dir)/upstream/libnfs && cmake . && make
> +
> +clean:
> +       cd $(module-dir) && rm -rf upstream
> diff --git a/modules/libnfs/usr.manifest b/modules/libnfs/usr.manifest
> --- a/modules/libnfs/usr.manifest
> +++ b/modules/libnfs/usr.manifest
> @@ -0,0 +1,9 @@
> +#
> +# Copyright (C) 2018 Waldemar Kozaczuk
> +#
> +# This work is open source software, licensed under the terms of the
> +# BSD license as described in the LICENSE file in the top-level directory.
> +#
> +
> +[manifest]
> +/usr/lib/libnfs.so.11.0.0:
> ${MODULE_DIR}/upstream/libnfs/lib/libnfs.so.4.0.0
> diff --git a/modules/nfs/.gitignore b/modules/nfs/.gitignore
> --- a/modules/nfs/.gitignore
> +++ b/modules/nfs/.gitignore
> @@ -0,0 +1,3 @@
> +obj
> +*.so
> +usr.manifest
> diff --git a/modules/nfs/Makefile b/modules/nfs/Makefile
> --- a/modules/nfs/Makefile
> +++ b/modules/nfs/Makefile
> @@ -0,0 +1,46 @@
> +INCLUDES = -I. -I../libnfs/upstream/libnfs/include -I../../include
> +INCLUDES += -I../../arch/$(ARCH) -I../.. -I../../build/$(mode)/gen/include
> +INCLUDES += -isystem ../../include/glibc-compat
> +#Only for host, not external
> +INCLUDES += $(shell $(CXX) -E -xc++ - -v </dev/null 2>&1 | awk '/^End/
> {exit} /^ .*c\+\+/ {print "-isystem" $$0}')
> +#
> +INCLUDES += -isystem ../../include/api -isystem ../../include/api/$(ARCH)
> -isystem ../../build/$(mode)/gen/include
> +INCLUDES += -isystem ../../bsd/sys -isystem ../../bsd/ -isystem
> ../../bsd/$(ARCH)
> +
> +autodepend = -MD -MT $@ -MP
> +CXXFLAGS  = -g -rdynamic -Wall -std=c++11 -fPIC $(INCLUDES) -D_KERNEL
> -D_GNU_SOURCE $(autodepend)
> +
> +# the build target executable:
> +TARGET = nfs
> +CPP_FILES := $(wildcard *.cc)
> +OBJ_FILES := $(addprefix obj/,$(CPP_FILES:.cc=.o))
> +DEPS := $(OBJ_FILES:.o=.d)
> +
> +LIBS = -L../libnfs/upstream/libnfs/lib -lnfs
> +ifndef ARCH
> +       ARCH = x64
> +endif
> +ifndef mode
> +       mode = release
> +endif
> +
> +quiet = $(if $V, $1, @echo " $2"; $1)
> +very-quiet = $(if $V, $1, @$1)
> +
> +$(TARGET).so: $(OBJ_FILES)
> +       $(call quiet, $(CXX) $(CXXFLAGS) -shared -o $(TARGET).so $^
> $(LIBS), LINK $@)
> +
> +obj/%.o: %.cc
> +       $(call quiet, $(CXX) $(CXXFLAGS) -c -o $@ $<, CXX $@)
> +
> +init:
> +       @echo "  MKDIRS"
> +       $(call very-quiet, mkdir -p obj)
> +.PHONY: init
> +
> +module: init $(TARGET).so
> +       echo '/usr/lib/fs/nfs.so: $${MODULE_DIR}/nfs.so' > usr.manifest
> +
> +clean:
> +       rm -f $(TARGET)*.so usr.manifest
> +       $(call very-quiet, $(RM) -rf obj)
> diff --git a/modules/nfs/module.py b/modules/nfs/module.py
> --- a/modules/nfs/module.py
> +++ b/modules/nfs/module.py
> @@ -0,0 +1,3 @@
> +from osv.modules import api
> +
> +api.require('libnfs')
> diff --git a/modules/nfs/nfs.cc b/modules/nfs/nfs.cc
> --- a/modules/nfs/nfs.cc
> +++ b/modules/nfs/nfs.cc
> @@ -31,7 +31,7 @@ mount_context::mount_context(const char *url)
>      // parse the url while taking care of freeing it when needed
>      _url.reset(nfs_parse_url_dir(_nfs.get(), url));
>      if (!_url) {
> -        debug(std::string("mount_context():g: ") +
> +        debug(std::string("mount_context(): ") +
>                nfs_get_error(_nfs.get()) + "\n");
>          _errno = EINVAL;
>          return;
> diff --git a/modules/nfs/nfs.hh b/modules/nfs/nfs.hh
> --- a/modules/nfs/nfs.hh
> +++ b/modules/nfs/nfs.hh
> @@ -27,7 +27,7 @@
>
>  #include <osv/mutex.h>
>
> -#include "../../external/fs/libnfs/include/nfsc/libnfs.h"
> +#include "nfsc/libnfs.h"
>
>  class mount_context {
>  public:
> diff --git a/modules/nfs/nfs_vfsops.cc b/modules/nfs/nfs_vfsops.cc
> --- a/modules/nfs/nfs_vfsops.cc
> +++ b/modules/nfs/nfs_vfsops.cc
> @@ -69,15 +69,17 @@ int nfs_init(void)
>  #define nfs_op_vget    ((vfsop_vget_t)vfs_nullop)
>  #define nfs_op_statfs  ((vfsop_statfs_t)vfs_nullop)
>
> -/*
> - * File system operations
> - */
> -struct vfsops nfs_vfsops = {
> -    nfs_op_mount,      /* mount */
> -    nfs_op_unmount,    /* unmount */
> -    nfs_op_sync,       /* sync */
> -    nfs_op_vget,       /* vget */
> -    nfs_op_statfs,     /* statfs */
> -    &nfs_vnops,        /* vnops */
> -};
> +// We are relying on vfsops structure defined in kernel
> +extern struct vfsops nfs_vfsops;
> +
> +// Overwrite "null" vfsops structure fields with "real"
> +// functions upon loading nfs.so shared object
> +void __attribute__((constructor)) initialize_vfsops() {
> +    nfs_vfsops.vfs_mount = nfs_op_mount;
> +    nfs_vfsops.vfs_unmount = nfs_op_unmount;
> +    nfs_vfsops.vfs_sync = nfs_op_sync;
> +    nfs_vfsops.vfs_vget = nfs_op_vget;
> +    nfs_vfsops.vfs_statfs = nfs_op_statfs;
> +    nfs_vfsops.vfs_vnops = &nfs_vnops;
> +}
>
> diff --git a/modules/nfs/nfs_vnops.cc b/modules/nfs/nfs_vnops.cc
> --- a/modules/nfs/nfs_vnops.cc
> +++ b/modules/nfs/nfs_vnops.cc
> null
> diff --git a/tools/mount/mount-fs.cc b/tools/mount/mount-fs.cc
> --- a/tools/mount/mount-fs.cc
> +++ b/tools/mount/mount-fs.cc
> @@ -9,28 +9,33 @@
>  int main(int argc, char **argv)
>  {
>      // Check number of arguments
> -    if (argc != 3) {
> +    if (argc != 4) {
>          std::cout << "Usage:" << std::endl;
>          std::cout << "\t" << argv[0] <<
> +                     " nfs" <<
>                       "
> nfs://<server|ipv4|ipv6>/path[?arg=val[&arg=val]*]" <<
>                       " /mount_point" << std::endl;
>          return(1);
>      }
>
>      // fetch arguments
> -    std::string url(argv[1]);
> -    std::string mount_point(argv[2]);
> +    std::string fs_type(argv[1]);
> +    std::string url(argv[2]);
> +    std::string mount_point(argv[3]);
>
>      // create the mount point as a convenience if it does not already
> exists
>      mkdir(mount_point.c_str(), 0777);
>      // Mount and process error
> -    int ret = mount(url.c_str(), mount_point.c_str(), "nfs", 0, nullptr);
> +    int ret = mount(url.c_str(), mount_point.c_str(), fs_type.c_str(), 0,
> nullptr);
>      if (ret) {
>          int my_errno = errno;
>          std::cout << "Error in mount(): " << strerror(my_errno) << "(" <<
> my_errno << ")"
>                    << std::endl;
>          return(1);
>      }
> +    else {
> +        std::cout << "Mounted " << url << " at " << mount_point <<
> std::endl;
> +    }
>
>      return(0);
>  }
> diff --git a/usr.manifest.skel b/usr.manifest.skel
> --- a/usr.manifest.skel
> +++ b/usr.manifest.skel
> @@ -7,7 +7,7 @@
>  /zfs.so: zfs.so
>  /tools/mkfs.so: tools/mkfs/mkfs.so
>  /tools/cpiod.so: tools/cpiod/cpiod.so
> -/tools/mount-nfs.so: tools/mount/mount-nfs.so
> +/tools/mount-fs.so: tools/mount/mount-fs.so
>  /tools/umount.so: tools/mount/umount.so
>  /usr/lib/libgcc_s.so.1: %(libgcc_s_dir)s/libgcc_s.so.1
>  /&/etc/hosts: ../../static/&
> diff --git a/usr_ramfs.manifest.skel b/usr_ramfs.manifest.skel
> --- a/usr_ramfs.manifest.skel
> +++ b/usr_ramfs.manifest.skel
> @@ -1,7 +1,7 @@
>  [manifest]
>  /libenviron.so: libenviron.so
>  /libvdso.so: libvdso.so
> -/tools/mount-nfs.so: tools/mount/mount-nfs.so
> +/tools/mount-fs.so: tools/mount/mount-fs.so
>  /tools/umount.so: tools/mount/umount.so
>  /usr/lib/libgcc_s.so.1: %(libgcc_s_dir)s/libgcc_s.so.1
>  /&/etc/hosts: ../../static/&
> diff --git a/usr_rofs.manifest.skel b/usr_rofs.manifest.skel
> --- a/usr_rofs.manifest.skel
> +++ b/usr_rofs.manifest.skel
> @@ -1,7 +1,7 @@
>  [manifest]
>  /libenviron.so: libenviron.so
>  /libvdso.so: libvdso.so
> -/tools/mount-nfs.so: tools/mount/mount-nfs.so
> +/tools/mount-fs.so: tools/mount/mount-fs.so
>  /tools/umount.so: tools/mount/umount.so
>  /usr/lib/libgcc_s.so.1: %(libgcc_s_dir)s/libgcc_s.so.1
>  /&/etc/hosts: ../../static/&
>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to osv-dev+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/osv-dev/00000000000053cb0105a1d7d801%40google.com
> .
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/osv-dev/CANEVyju%2BSHTqpsuj6hzWv%2B2pOM%3DJ%3D2anNzbHYiQuTSvNnMZZvg%40mail.gmail.com.

Reply via email to