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

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 29.08.19 15:43, Costin Lupu wrote:
> Hi Simon,
>
> On 8/29/19 4:17 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 /"
> I'd suggest LIBUKBOOT_RAMFSROOT or smth like that since ramfs != vfs.
Fine, we can change this. I don't mind.

> > +       default n
> > +       select LIBRAMFS
> >   endif
> > diff --git a/lib/ukboot/boot.c b/lib/ukboot/boot.c
> > index b4b390ef..15954205 100644
> > --- a/lib/ukboot/boot.c
> > +++ b/lib/ukboot/boot.c
> > @@ -64,6 +64,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
> > @@ -90,6 +95,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");
> Why do we restrict ukboot to using ramfs? Why not keeping a constructor
> in the ramfs lib instead?
I put it here in order (1) to gain more flexibility in the future and 
(2) to keep code for mounting the initial fs'es at a single place 
instead all over the place with constructors. As soon as we have a 
Unikraft inittab, we could move this to a init function instead.
I also have in mind that we could use Sharan's library parameters to 
specify something else than a ramfs root - like 9pfs as root mount 
point. I could imagine something like `ukboot.root=9pfs:sharename`, 
`ukboot.root=ramfs`, or `ukboot.root=initrd` as example. It would be 
easier to handle this here instead of enabling/disabling individual 
constructors.
I think it is also the right place as soon as we want to support 
initramdisks which could be an .tar.gz (or something else) that we would 
extract to the ramfs root. As soon as you are done, you are able to 
claim the initrd memory region to the memory allocator. Its space is not 
needed anymore for something else. For this purpose this file is also 
the right place, the allocator is initialized here and memory regions 
are scanned here.
> > +
> > +       /*
> > +        * 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