Re: [Minios-devel] [UNIKRAFT PATCH v2 2/6] lib/devfs, lib/ukboot: Move option to automatically mount root to libukboot

[Costin Lupu <costin.lup@xxxxxxxxx>]

Hi Simon,

The general comment in patch 6/6 would apply here as well. It would be
better if we would have this kind of logic for initializing the
filesystem elsewhere, maybe in vfscore in a new file.

On 9/6/19 3:03 PM, Simon Kuenzer wrote:
> Moves the option of automatically mounting ramfs as a root filesystem
> to lib/ukboot. This feature is not only useful for initializing
> devfs. The idea is that we could later feed its initial content from a
> archive given through an initrd.
> 
> Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> ---
>  lib/devfs/Config.uk     |  8 +-------
>  lib/devfs/devfs_vnops.c | 25 -------------------------
>  lib/ukboot/Config.uk    |  5 +++++
>  lib/ukboot/boot.c       | 28 ++++++++++++++++++++++++++++
>  4 files changed, 34 insertions(+), 32 deletions(-)
> 
> diff --git a/lib/devfs/Config.uk b/lib/devfs/Config.uk
> index e38a616e..f6b16aee 100644
> --- a/lib/devfs/Config.uk
> +++ b/lib/devfs/Config.uk
> @@ -1,10 +1,4 @@
>  config LIBDEVFS
>       bool "devfs: devfs file system"
>       default n
> -     depends on LIBVFSCORE
> -if LIBDEVFS
> -        config LIBDEVFS_USE_RAMFS
> -        bool "Use ramfs as root"
> -        default n
> -        select LIBRAMFS
> -endif
> +     select LIBVFSCORE
> diff --git a/lib/devfs/devfs_vnops.c b/lib/devfs/devfs_vnops.c
> index 11a3ea05..3399b2bc 100644
> --- a/lib/devfs/devfs_vnops.c
> +++ b/lib/devfs/devfs_vnops.c
> @@ -309,28 +309,3 @@ static struct vfscore_fs_type fs_devfs = {
>  };
>  
>  UK_FS_REGISTER(fs_devfs);
> -
> -__constructor_prio(101) static void devfs_init(void)
> -{
> -#ifdef CONFIG_LIBDEVFS_USE_RAMFS
> -     int ret;
> -
> -     ret = mount("", "/", "ramfs", 0, NULL);
> -     if (ret != 0) {
> -             uk_pr_debug("Failed to mount / in %s\n", __func__);
> -             return;
> -     }
> -
> -     ret =  mkdir("/dev", S_IRWXU);
> -     if (ret != 0) {
> -             uk_pr_debug("Failed to mkdir /dev in %s\n", __func__);
> -             return;
> -     }
> -
> -     ret = mount("", "/dev", "devfs", 0, NULL);
> -     if (ret != 0) {
> -             uk_pr_debug("Failed to mount /dev as devfs in %s\n", __func__);
> -             return;
> -     }
> -#endif
> -}
> diff --git a/lib/ukboot/Config.uk b/lib/ukboot/Config.uk
> index 841a8767..6f86c7de 100644
> --- a/lib/ukboot/Config.uk
> +++ b/lib/ukboot/Config.uk
> @@ -21,4 +21,9 @@ if LIBUKBOOT
>       bool "Initialize ukallocbbuddy as allocator"
>       default y
>       select LIBUKALLOCBBUDDY
> +
> +     config LIBUKBOOT_VFSROOT
> +     bool "Mount ramfs to /"
> +     default n
> +     select LIBRAMFS
>  endif
> diff --git a/lib/ukboot/boot.c b/lib/ukboot/boot.c
> index 7fb0908a..cb3403e1 100644
> --- a/lib/ukboot/boot.c
> +++ b/lib/ukboot/boot.c
> @@ -63,6 +63,11 @@
>  #ifdef CONFIG_LIBUKLIBPARAM
>  #include <uk/libparam.h>
>  #endif /* CONFIG_LIBUKLIBPARAM */
> +#ifdef CONFIG_LIBUKBOOT_VFSROOT
> +#include <sys/stat.h>
> +#include <sys/mount.h>
> +#endif /* CONFIG_LIBUKBOOT_VFSROOT */
> +
>  
>  int main(int argc, char *argv[]) __weak;
>  #ifdef CONFIG_LIBLWIP
> @@ -89,6 +94,29 @@ static void main_thread_func(void *arg)
>       uk_bus_probe_all();
>  #endif /* CONFIG_LIBUKBUS */
>  
> +     /*
> +      * VFS initialization
> +      */
> +#ifdef CONFIG_LIBUKBOOT_VFSROOT
> +     /*
> +      * TODO: Provide a boot parameter option to specify a custom
> +      * root mount (e.g., ramfs, initrd, 9pfs).
> +      */
> +     uk_pr_info("Mount root...\n");
> +     ret = mount("", "/", "ramfs", 0, NULL);
> +     if (ret != 0)
> +             UK_CRASH("Failed to mount ramfs to /\n");

Given that now we also have 9pfs, we can't apply this kind of solution
anymore. We should think about a more scalable one to support multiple
filesystems types. More than that, it's very confusing that the changes
of this patch are later touched by patches in this series, so please
make the necessary changes only once.

> +
> +     /*
> +      * TODO: We could place here code that extracts an archive
> +      * found as initrd to '/'
> +      */
> +
> +#endif /* CONFIG_LIBUKBOOT_VFSROOT */
> +
> +     /*
> +      * Network initialization
> +      */
>  #ifdef CONFIG_LIBLWIP
>       /*
>        * TODO: This is an initial implementation where we call the
> 

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel