VFS file and directory deletion

[SamTebbs33]

The virtual filesystem should support deleting of files and directories (which should only be deletable if it has no children).

[iamgweej]

How will this work? Should initrd support file deletion? Or should an Unimplemented error be added to vfs.Error?

[SamTebbs33]

My opinion is that whether deletion is implemented shouldn't be a concern for the code attempting to delete a file, so I don't think it should throw an error. I think it would be fine to just do nothing and perhaps log that an unsupported operation was attempted. As the implementer, @DrDeano may have some input.

[DrDeano]

For the initrd, this isn't a "real" file system, so probrably just log something at the most, but will just do nothing. As the user won't be interacting with the initrd anyway, there won't be any code that trys to delete a file from the initrd.

[iamgweej]

I've been looking a bit into old linux kernels to see how to implement their VFS, and it seems to look something like this:

struct file_operations {
	int (*lseek) (struct inode *, struct file *, off_t, int);
	int (*read) (struct inode *, struct file *, char *, int);
	int (*write) (struct inode *, struct file *, const char *, int);
	int (*readdir) (struct inode *, struct file *, void *, filldir_t);
	int (*select) (struct inode *, struct file *, int, select_table *);
	int (*ioctl) (struct inode *, struct file *, unsigned int, unsigned long);
	int (*mmap) (struct inode *, struct file *, struct vm_area_struct *);
	int (*open) (struct inode *, struct file *);
	void (*release) (struct inode *, struct file *);
	int (*fsync) (struct inode *, struct file *);
	int (*fasync) (struct inode *, struct file *, int);
	int (*check_media_change) (kdev_t dev);
	int (*revalidate) (kdev_t dev);
};

and:

struct inode_operations {
	struct file_operations * default_file_ops;
	int (*create) (struct inode *,const char *,int,int,struct inode **);
	int (*lookup) (struct inode *,const char *,int,struct inode **);
	int (*link) (struct inode *,struct inode *,const char *,int);
	int (*unlink) (struct inode *,const char *,int);
	int (*symlink) (struct inode *,const char *,int,const char *);
	int (*mkdir) (struct inode *,const char *,int,int);
	int (*rmdir) (struct inode *,const char *,int);
	int (*mknod) (struct inode *,const char *,int,int,int);
	int (*rename) (struct inode *,const char *,int,struct inode *,const char *,int);
	int (*readlink) (struct inode *,char *,int);
	int (*follow_link) (struct inode *,struct inode *,int,int,struct inode **);
	int (*readpage) (struct inode *, struct page *);
	int (*writepage) (struct inode *, struct page *);
	int (*bmap) (struct inode *,int);
	void (*truncate) (struct inode *);
	int (*permission) (struct inode *, int);
	int (*smap) (struct inode *,int);
};

I don't think we should feel obligated to follow their implementation, but I think it gives a rough idea what our VFS should support and in what fashion.

[iamgweej]

It looks like their implementation (linux 2.0) supports deleting a file from a directory, as an operation on the directory taking the file basename as a parameter.

I've also checked in a newer kernel (v5.9-rc2) and it seems that there it is also an operation on the directory containing the file: https://www.kernel.org/doc/htmldocs/filesystems/API-vfs-unlink.html and https://elixir.bootlin.com/linux/v5.9-rc2/source/fs/namei.c#L3812

[iamgweej]

Maybe something like this?

    ///
    /// Delete a member of this filesystem.
    ///
    /// Arguments:
    ///     IN self: *const FileSystem - The filesystem in question being operated on
    ///     IN node: *const DirNode - The directory to delete a file/directory from
    ///     IN name: []const u8 - The name of the file/directory to delete
    ///
    /// Error: Allocator.Error || Error
    ///     Allocator.Error.OutOfMemory - There wasn't enough memory to fulfill the request
    ///     Error.NoSuchFileOrDir - The file/dir by that name doesn't exist.
    ///     Error.NonemptyDir - The directory by that name is not empty, and therefore it can't be deleted.
    ///
    const Delete = fn (self: *const Self, node: *const DirNode, name: []const u8) (Allocator.Error || Error)!void;

[SamTebbs33]

That looks good to me! It's nice that it is similar to open. Taking the name to represent the file/dir being deleted rather than a node is a good idea since it shouldn't have to be necessary to open a file/dir in order to delete it.