Re: [Minios-devel] [UNIKRAFT PATCH v3 4/5] lib/devfs: Automount through inittab

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 27.09.19 19:00, Sharan Santhanam wrote:
> Hello Simon,
>
> Please find the comment inline.
>
> Thanks & Regards
> Sharan
>
> On 9/24/19 12:07 PM, Simon Kuenzer wrote:
> > The automount option of devfs to `/dev` is moved to an inittab entry.
> >
> > Signed-off-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> > ---
> >   lib/devfs/devfs_vnops.c          | 29 ++++++++++++++++++++---------
> >   lib/devfs/include/devfs/device.h |  3 +++
> >   2 files changed, 23 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/devfs/devfs_vnops.c b/lib/devfs/devfs_vnops.c
> > index d872e622..278a1a61 100644
> > --- a/lib/devfs/devfs_vnops.c
> > +++ b/lib/devfs/devfs_vnops.c
> > @@ -59,7 +59,8 @@
> >   #include <vfscore/fs.h>
> > -#include <uk/ctors.h>
> > +#include <uk/init.h>
> > +#include <uk/print.h>
> >   #include "devfs.h"
> >   #include <devfs/device.h>
> > @@ -310,21 +311,31 @@ static struct vfscore_fs_type fs_devfs = {
> >   UK_FS_REGISTER(fs_devfs);
> > -__constructor_prio(101) static void devfs_init(void)
> > -{
> >   #ifdef CONFIG_LIBDEVFS_AUTOMOUNT
> > +static int devfs_automount(void)
> > +{
> >       int ret;
> > +    uk_pr_info("Mount devfs to /dev...");
> > +
> > +    /*
> > +     * Try to create target mountpoint `/dev`. If creation fails
> > +     * because it already exists, we are continuing.
> > +     */
> >       ret =  mkdir("/dev", S_IRWXU);
> > -    if (ret != 0) {
> > -        uk_pr_debug("Failed to mkdir /dev in %s\n", __func__);
> > -        return;
> > +    if (ret != 0 && errno != EEXIST) {
> > +        uk_pr_err("Failed to create /dev: %d\n", errno);
> > +        return -1;
> >       }
> >       ret = mount("", "/dev", "devfs", 0, NULL);
> >       if (ret != 0) {
> > -        uk_pr_debug("Failed to mount /dev as devfs in %s\n", __func__);
> > -        return;
> > +        uk_pr_err("Failed to mount devfs to /dev: %d\n", errno);
> > +        return -1;
> >       }
> > -#endif
> > +
> > +    return 0;
> >   }
> > +
> > +uk_rootfs_initcall_prio(devfs_automount, 1);
> > +#endif
> > diff --git a/lib/devfs/include/devfs/device.h 
> > b/lib/devfs/include/devfs/device.h
> > index 29889e4e..569ef60b 100644
> > --- a/lib/devfs/include/devfs/device.h
> > +++ b/lib/devfs/include/devfs/device.h
> > @@ -34,6 +34,7 @@
> >   #define _DEVICE_H
> >   #include <sys/types.h>
> > +#include <uk/init.h>
> >   #include <vfscore/uio.h>
> > @@ -208,4 +209,6 @@ int device_destroy_locked(struct device *dev);
> >   void device_register(struct device *device, const char *name, int 
> > flags);
> >   void read_partition_table(struct device *device);
> > +#define devfs_initcall(fn) uk_lib_initcall(fn)
> Why do we need this? We could add documentation of this API.
I needed something to have initcalls like rootfs-1 which is the 
lib_initcall actually. As we spoke offline this may be still a bad idea 
because of potential dependency issues. As we discussed it would be 
better to put that into the rootfs stage but move mounting of root from 
0 to 1 or 2 in that priority band.
I will also put add a comment what this define is for.

> > +
> >   #endif /* !_DEVICE_H */
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel