|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH 3/6] lib/devfs: Adapt imported devfs to Unikraft
Hi Vlad,
Please see my comment inline.
On 4/17/19 2:32 PM, Vlad-Andrei BĂDOIU (78692) wrote:
> Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
> ---
> lib/devfs/devfs.h | 2 +-
> lib/devfs/devfs_vnops.c | 122 ++++++++++++---
> lib/devfs/device.c | 218 +++++----------------------
> lib/vfscore/include/vfscore/device.h | 9 +-
> 4 files changed, 141 insertions(+), 210 deletions(-)
>
> diff --git a/lib/devfs/devfs.h b/lib/devfs/devfs.h
> index bdf50609..c2197f79 100644
> --- a/lib/devfs/devfs.h
> +++ b/lib/devfs/devfs.h
> @@ -30,7 +30,7 @@
> #ifndef _DEVFS_H
> #define _DEVFS_H
>
> -#include <assert.h>
> +#include <uk/assert.h>
>
> /* #define DEBUG_DEVFS 1 */
>
> diff --git a/lib/devfs/devfs_vnops.c b/lib/devfs/devfs_vnops.c
> index 1e69e4a6..1ec0d553 100644
> --- a/lib/devfs/devfs_vnops.c
> +++ b/lib/devfs/devfs_vnops.c
> @@ -31,6 +31,10 @@
> * devfs - device file system.
> */
>
> +#ifndef _GNU_SOURCE
> +#define _GNU_SOURCE
> +#endif
> +
> #include <sys/stat.h>
>
> #include <ctype.h>
> @@ -41,27 +45,34 @@
> #include <limits.h>
> #include <fcntl.h>
> #include <stdio.h>
> +#include <sys/mount.h>
> +
> +#include <dirent.h>
> +#include <vfscore/prex.h>
> +#include <vfscore/device.h>
> +#include <vfscore/vnode.h>
> +#include <vfscore/mount.h>
> +#include <vfscore/dentry.h>
> +#include <vfscore/file.h>
>
> -#include <osv/prex.h>
> -#include <osv/device.h>
> -#include <osv/vnode.h>
> -#include <osv/mount.h>
> -#include <osv/dentry.h>
> +#include <vfscore/fs.h>
>
> #include "devfs.h"
>
> +#include <uk/ctors.h>
> +
> #ifdef DEBUG_DEVFS
> -#define DPRINTF(a) dprintf a
> +#define DPRINTF(X) uk_pr_debug X
> #else
> -#define DPRINTF(a) do {} while (0)
> +#define DPRINTF(X)
> #endif
>
> -#define ASSERT(e) assert(e)
> +#define ASSERT(e)
>
> static uint64_t inode_count = 1; /* inode 0 is reserved to root */
>
> static int
> -devfs_open(struct file *fp)
> +devfs_open(struct vfscore_file *fp)
> {
> struct vnode *vp = fp->f_dentry->d_vnode;
> char *path = fp->f_dentry->d_path;
> @@ -90,7 +101,7 @@ devfs_open(struct file *fp)
> }
>
> static int
> -devfs_close(struct vnode *vp, struct file *fp)
> +devfs_close(struct vnode *vp, struct vfscore_file *fp)
> {
>
> DPRINTF(("devfs_close: fp=%x\n", fp));
> @@ -98,27 +109,27 @@ devfs_close(struct vnode *vp, struct file *fp)
> if (!strcmp(fp->f_dentry->d_path, "/")) /* root ? */
> return 0;
>
> - return device_close((device*)vp->v_data);
> + return device_close((struct device*)vp->v_data);
> }
>
> static int
> -devfs_read(struct vnode *vp, struct file *fp, struct uio *uio, int ioflags)
> +devfs_read(struct vnode *vp, struct vfscore_file *fp, struct uio *uio, int
> ioflags)
> {
> - return device_read((device*)vp->v_data, uio, ioflags);
> + return device_read((struct device*)vp->v_data, uio, ioflags);
> }
>
> static int
> devfs_write(struct vnode *vp, struct uio *uio, int ioflags)
> {
> - return device_write((device*)vp->v_data, uio, ioflags);
> + return device_write((struct device*)vp->v_data, uio, ioflags);
> }
>
> static int
> -devfs_ioctl(struct vnode *vp, struct file *fp, u_long cmd, void *arg)
> +devfs_ioctl(struct vnode *vp, struct vfscore_file *fp, u_long cmd, void *arg)
> {
> int error;
>
> - error = device_ioctl((device*)vp->v_data, cmd, arg);
> + error = device_ioctl((struct device*)vp->v_data, cmd, arg);
> DPRINTF(("devfs_ioctl: cmd=%x\n", cmd));
> return error;
> }
> @@ -149,7 +160,7 @@ devfs_lookup(struct vnode *dvp, char *name, struct vnode
> **vpp)
> break;
> i++;
> }
> - if (vget(dvp->v_mount, inode_count++, &vp)) {
> + if (vfscore_vget(dvp->v_mount, inode_count++, &vp)) {
> /* found in cache */
> *vpp = vp;
> return 0;
> @@ -171,7 +182,7 @@ devfs_lookup(struct vnode *dvp, char *name, struct vnode
> **vpp)
> * @vp: vnode of the directory.
> */
> static int
> -devfs_readdir(struct vnode *vp, struct file *fp, struct dirent *dir)
> +devfs_readdir(struct vnode *vp, struct vfscore_file *fp, struct dirent *dir)
> {
> struct devinfo info;
> int error, i;
> @@ -204,15 +215,10 @@ devfs_readdir(struct vnode *vp, struct file *fp, struct
> dirent *dir)
> static int
> devfs_unmount(struct mount *mp, int flags)
> {
> - release_mp_dentries(mp);
> + //release_mp_dentries(mp);
> return 0;
> }
>
> -int
> -devfs_init(void)
> -{
> - return 0;
> -}
>
> static int
> devfs_getattr(struct vnode *vnode, struct vattr *attr)
> @@ -222,6 +228,39 @@ devfs_getattr(struct vnode *vnode, struct vattr *attr)
> return 0;
> }
>
> +int
> +vop_nullop(void)
> +{
> + return 0;
> +}
> +
> +int
> +vop_einval(void)
> +{
> + return EINVAL;
> +}
> +
> +int
> +vop_eperm(void)
> +{
> + return EPERM;
> +}
> +
> +int
> +vop_erofs(void)
> +{
> + return EROFS;
> +}
> +
> +int
> +vfs_nullop(void)
> +{
> + return 0;
> +}
> +
> +
> +
> +
> #define devfs_mount ((vfsop_mount_t)vfs_nullop)
> #define devfs_sync ((vfsop_sync_t)vfs_nullop)
> #define devfs_vget ((vfsop_vget_t)vfs_nullop)
> @@ -265,7 +304,7 @@ struct vnops devfs_vnops = {
> devfs_inactive, /* inactive */
> devfs_truncate, /* truncate */
> devfs_link, /* link */
> - (vnop_cache_t) nullptr, /* arc */
> + (vnop_cache_t) NULL, /* arc */
> devfs_fallocate, /* fallocate */
> devfs_readlink, /* read link */
> devfs_symlink, /* symbolic link */
> @@ -282,3 +321,36 @@ struct vfsops devfs_vfsops = {
> devfs_statfs, /* statfs */
> &devfs_vnops, /* vnops */
> };
> +
> +static struct vfscore_fs_type fs_devfs = {
> + .vs_name = "devfs",
> + .vs_init = NULL,
> + .vs_op = &devfs_vfsops,
> +};
> +
> +UK_FS_REGISTER(fs_devfs);
> +
> +
> +__constructor_prio(101) static void devfs_init(void)
> +{
> + int ret;
> +
> + ret = mount("", "/", "ramfs", 0, NULL);
I don't think devfs_init() should mount the root filesystem. Moreover,
the whole logic of this function looks like something that should be
called from the application, it looks more of a generic fs_init() to me.
To be more clear, I want to be able to mount a different filesystem type
for root, say ext4 type, and after that mount defs.
> + if (ret != 0) {
> + DPRINTF(("Failed to mount / in %s\n", __func__));
> + return;
> + }
> +
> + ret = mkdir("/dev", S_IRWXU);
> + if (ret != 0) {
> + DPRINTF(("Failed to mkdir /dev in %s\n", __func__));
> + return;
> + }
> +
> + ret = mount("", "/dev", "devfs", 0, NULL);
> + if (ret != 0) {
> + DPRINTF(("Failed to mount /dev as devfs in %s\n", __func__));
> + return;
> + }
> +
> +}
> diff --git a/lib/devfs/device.c b/lib/devfs/device.c
> index a0cd9768..50522076 100644
> --- a/lib/devfs/device.c
> +++ b/lib/devfs/device.c
> @@ -45,21 +45,15 @@
> #include <stdio.h>
> #include <unistd.h>
> #include <errno.h>
> -#include <assert.h>
> +#include <uk/assert.h>
> #include <string.h>
>
> -#include <osv/prex.h>
> -#include <osv/mutex.h>
> -#include <osv/device.h>
> -#include <osv/debug.h>
> -#include <osv/buf.h>
> -
> -#include <geom/geom_disk.h>
> -
> -mutex sched_mutex;
> -#define sched_lock() sched_mutex.lock()
> -#define sched_unlock() sched_mutex.unlock()
> +#include <vfscore/device.h>
> +#include <vfscore/prex.h>
> +#include <uk/essentials.h>
> +#include <uk/mutex.h>
>
> +static struct uk_mutex devfs_lock = UK_MUTEX_INITIALIZER(devfs_lock);
>
> /* list head of the devices */
> static struct device *device_list = NULL;
> @@ -92,85 +86,31 @@ struct partition_table_entry {
> uint32_t total_sectors;
> } __attribute__((packed));
>
> -/*
> - * read_partition_table - given a device @dev, create one subdevice per
> partition
> - * found in that device.
> - *
> - * This function will read a partition table from the canonical location of
> the
> - * device pointed by @dev. For each partition found, a new device will be
> - * created. The newly created device will have most of its data copied from
> - * @dev, except for its name, offset and size.
> - */
> -void read_partition_table(struct device *dev)
> -{
> - struct buf *bp;
> - unsigned long offset;
> - int index;
> -
> - if (bread(dev, 0, &bp) != 0) {
> - debugf("read_partition_table failed for %s\n", dev->name);
> - return;
> - }
> -
> - sched_lock();
> - // A real partition table (MBR) ends in the two bytes 0x55, 0xAA (see
> - // arch/x64/boot16.S on where we put those on the OSv image). If we
> - // don't find those, this is an unpartitioned disk.
> - if (((unsigned char*)bp->b_data)[510] == 0x55 &&
> - ((unsigned char*)bp->b_data)[511] == 0xAA)
> - for (offset = 0x1be, index = 0; offset < 0x1fe; offset += 0x10,
> index++) {
> - char dev_name[MAXDEVNAME];
> - struct device *new_dev;
> -
> - auto* entry = static_cast<struct
> partition_table_entry*>(bp->b_data + offset);
> -
> - if (entry->system_id == 0) {
> - continue;
> - }
> -
> - if (entry->starting_sector == 0) {
> - continue;
> - }
> -
> - snprintf(dev_name, MAXDEVNAME, "%s.%d", dev->name, index);
> - new_dev = device_create(dev->driver, dev_name, dev->flags);
> - free(new_dev->private_data);
> -
> - new_dev->offset = (off_t)entry->rela_sector << 9;
> - new_dev->size = (off_t)entry->total_sectors << 9;
> - new_dev->max_io_size = dev->max_io_size;
> - new_dev->private_data = dev->private_data;
> - device_set_softc(new_dev, device_get_softc(dev));
> - }
> -
> - sched_unlock();
> - brelse(bp);
> -}
>
> void device_register(struct device *dev, const char *name, int flags)
> {
> size_t len;
> void *priv = NULL;
> -
> - assert(dev->driver != NULL);
> +
> + UK_ASSERT(dev->driver != NULL);
>
> /* Check the length of name. */
> len = strnlen(name, MAXDEVNAME);
> if (len == 0 || len >= MAXDEVNAME)
> return;
>
> - sched_lock();
> + uk_mutex_lock(&devfs_lock);
>
> /* Check if specified name is already used. */
> - if (device_lookup(name) != NULL)
> - sys_panic("duplicate device");
> + //if (device_lookup(name) != NULL)
> + //sys_panic("duplicate device");
>
> /*
> * Allocate a device and device private data.
> */
> if (dev->driver->devsz != 0) {
> if ((priv = malloc(dev->driver->devsz)) == NULL)
> - sys_panic("devsz");
> + UK_CRASH("devsz");
> memset(priv, 0, dev->driver->devsz);
> }
>
> @@ -183,8 +123,8 @@ void device_register(struct device *dev, const char
> *name, int flags)
> dev->next = device_list;
> dev->max_io_size = UINT_MAX;
> device_list = dev;
> -
> - sched_unlock();
> +
> + uk_mutex_unlock(&devfs_lock);
> }
>
>
> @@ -201,7 +141,7 @@ device_create(struct driver *drv, const char *name, int
> flags)
> struct device *dev;
> size_t len;
>
> - assert(drv != NULL);
> + UK_ASSERT(drv != NULL);
>
> /* Check the length of name. */
> len = strnlen(name, MAXDEVNAME);
> @@ -211,27 +151,14 @@ device_create(struct driver *drv, const char *name, int
> flags)
> /*
> * Allocate a device structure.
> */
> - if ((dev = new device) == NULL)
> - sys_panic("device_create");
> + if ((dev = malloc(sizeof(struct device))) == NULL)
> + UK_CRASH("device_create");
>
> dev->driver = drv;
> device_register(dev, name, flags);
> return dev;
> }
>
> -#if 0
> -/*
> - * Return device's private data.
> - */
> -static void *
> -device_private(struct device *dev)
> -{
> - assert(dev != NULL);
> - assert(dev->private_data != NULL);
> -
> - return dev->private_data;
> -}
> -#endif
>
> /*
> * Return true if specified device is valid.
> @@ -260,13 +187,13 @@ static int
> device_reference(struct device *dev)
> {
>
> - sched_lock();
> + uk_mutex_lock(&devfs_lock);
> if (!device_valid(dev)) {
> - sched_unlock();
> + uk_mutex_unlock(&devfs_lock);
> return ENODEV;
> }
> dev->refcnt++;
> - sched_unlock();
> + uk_mutex_unlock(&devfs_lock);
> return 0;
> }
>
> @@ -279,9 +206,10 @@ device_release(struct device *dev)
> {
> struct device **tmp;
>
> - sched_lock();
> + uk_mutex_lock(&devfs_lock);
> +
> if (--dev->refcnt > 0) {
> - sched_unlock();
> + uk_mutex_unlock(&devfs_lock);
> return;
> }
> /*
> @@ -293,22 +221,22 @@ device_release(struct device *dev)
> break;
> }
> }
> - delete dev;
> - sched_unlock();
> + free(dev);
> + uk_mutex_unlock(&devfs_lock);
> }
>
> int
> device_destroy(struct device *dev)
> {
>
> - sched_lock();
> + uk_mutex_lock(&devfs_lock);
> if (!device_valid(dev)) {
> - sched_unlock();
> + uk_mutex_unlock(&devfs_lock);
> return ENODEV;
> }
> dev->active = 0;
> + uk_mutex_unlock(&devfs_lock);
> device_release(dev);
> - sched_unlock();
> return 0;
> }
>
> @@ -329,20 +257,20 @@ device_open(const char *name, int mode, struct device
> **devp)
> struct device *dev;
> int error;
>
> - sched_lock();
> + uk_mutex_lock(&devfs_lock);
> if ((dev = device_lookup(name)) == NULL) {
> - sched_unlock();
> + uk_mutex_unlock(&devfs_lock);
> return ENXIO;
> }
> + uk_mutex_unlock(&devfs_lock);
> +
> error = device_reference(dev);
> if (error) {
> - sched_unlock();
> return error;
> }
> - sched_unlock();
>
> ops = dev->driver->devops;
> - assert(ops->open != NULL);
> + UK_ASSERT(ops->open != NULL);
> error = (*ops->open)(dev, mode);
> *devp = dev;
>
> @@ -366,7 +294,7 @@ device_close(struct device *dev)
> return error;
>
> ops = dev->driver->devops;
> - assert(ops->close != NULL);
> + UK_ASSERT(ops->close != NULL);
> error = (*ops->close)(dev);
>
> device_release(dev);
> @@ -383,7 +311,7 @@ device_read(struct device *dev, struct uio *uio, int
> ioflags)
> return error;
>
> ops = dev->driver->devops;
> - assert(ops->read != NULL);
> + UK_ASSERT(ops->read != NULL);
> error = (*ops->read)(dev, uio, ioflags);
>
> device_release(dev);
> @@ -400,7 +328,7 @@ device_write(struct device *dev, struct uio *uio, int
> ioflags)
> return error;
>
> ops = dev->driver->devops;
> - assert(ops->write != NULL);
> + UK_ASSERT(ops->write != NULL);
> error = (*ops->write)(dev, uio, ioflags);
>
> device_release(dev);
> @@ -424,80 +352,13 @@ device_ioctl(struct device *dev, u_long cmd, void *arg)
> return error;
>
> ops = dev->driver->devops;
> - assert(ops->ioctl != NULL);
> + UK_ASSERT(ops->ioctl != NULL);
> error = (*ops->ioctl)(dev, cmd, arg);
>
> device_release(dev);
> return error;
> }
>
> -#if 0
> -/*
> - * Device control - devctl is similar to ioctl, but is invoked from
> - * other device driver rather than from user application.
> - */
> -static int
> -device_control(struct device *dev, u_long cmd, void *arg)
> -{
> - struct devops *ops;
> - int error;
> -
> - assert(dev != NULL);
> -
> - sched_lock();
> - ops = dev->driver->devops;
> - assert(ops->devctl != NULL);
> - error = (*ops->devctl)(dev, cmd, arg);
> - sched_unlock();
> - return error;
> -}
> -
> -/*
> - * device_broadcast - broadcast devctl command to all device objects.
> - *
> - * If "force" argument is true, we will continue command
> - * notification even if some driver returns an error. In this
> - * case, this routine returns EIO error if at least one driver
> - * returns an error.
> - *
> - * If force argument is false, a kernel stops the command processing
> - * when at least one driver returns an error. In this case,
> - * device_broadcast will return the error code which is returned
> - * by the driver.
> - */
> -static int
> -device_broadcast(u_long cmd, void *arg, int force)
> -{
> - struct device *dev;
> - struct devops *ops;
> - int error, retval = 0;
> -
> - sched_lock();
> -
> - for (dev = device_list; dev != NULL; dev = dev->next) {
> - /*
> - * Call driver's devctl() routine.
> - */
> - ops = dev->driver->devops;
> - if (ops == NULL)
> - continue;
> -
> - assert(ops->devctl != NULL);
> - error = (*ops->devctl)(dev, cmd, arg);
> - if (error) {
> - if (force)
> - retval = EIO;
> - else {
> - retval = error;
> - break;
> - }
> - }
> - }
> - sched_unlock();
> - return retval;
> -}
> -#endif
> -
> /*
> * Return device information.
> */
> @@ -509,7 +370,7 @@ device_info(struct devinfo *info)
> struct device *dev;
> int error = ESRCH;
>
> - sched_lock();
> + uk_mutex_lock(&devfs_lock);
> for (dev = device_list; dev != NULL; dev = dev->next) {
> if (i++ == target) {
> info->cookie = i;
> @@ -520,7 +381,7 @@ device_info(struct devinfo *info)
> break;
> }
> }
> - sched_unlock();
> + uk_mutex_unlock(&devfs_lock);
> return error;
> }
>
> @@ -535,3 +396,4 @@ nullop(void)
> {
> return 0;
> }
> +
> diff --git a/lib/vfscore/include/vfscore/device.h
> b/lib/vfscore/include/vfscore/device.h
> index 16d2e470..bcb33c33 100644
> --- a/lib/vfscore/include/vfscore/device.h
> +++ b/lib/vfscore/include/vfscore/device.h
> @@ -30,16 +30,15 @@
> #ifndef _DEVICE_H
> #define _DEVICE_H
>
> -#include <sys/cdefs.h>
> #include <sys/types.h>
>
> -#include <osv/uio.h>
> -
> -__BEGIN_DECLS
> +#include <vfscore/uio.h>
>
> #define MAXDEVNAME 12
> #define DO_RWMASK 0x3
>
> +typedef unsigned long u_long;
> +
> struct bio;
> struct device;
>
> @@ -208,6 +207,4 @@ int device_destroy(struct device *dev);
> void device_register(struct device *device, const char *name, int flags);
> void read_partition_table(struct device *device);
>
> -__END_DECLS
> -
> #endif /* !_DEVICE_H */
>
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |