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
[email protected]


On Fri, Mar 27, 2020 at 6:35 PM Commit Bot <[email protected]> wrote:

> From: Waldemar Kozaczuk <[email protected]>
> Committer: Waldemar Kozaczuk <[email protected]>
> 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 <[email protected]>
>
> ---
> 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 [email protected].
> 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 [email protected].
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