This patch adds ioctls to vfs for compatibility with legacy XFS
pre-allocation ioctls (XFS_IOC_*RESVP*). The implementation
effectively invokes sys_fallocate for the new ioctls.
Note: These legacy ioctls are also implemented by OCFS2.
There are some things that I'm not sure about:
1. Should the struct space_resv be exposed to user-space? If not,
then what would be the right place to put it? And the ioctl
definitions?
2. Should the corresponding ioctls be removed from ocfs2?
Signed-off-by: Ankit Jain <[email protected]>
---
fs/ioctl.c | 37 +++++++++++++++++++++++++++
fs/open.c | 51 ++++++++++++++++++-------------------
include/linux/falloc.h | 19 ++++++++++++++
include/linux/fs.h | 2 +
4 files changed, 83 insertions(+), 26 deletions(-)
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 43e8b2c..5e565c8 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -15,6 +15,7 @@
#include <linux/uaccess.h>
#include <linux/writeback.h>
#include <linux/buffer_head.h>
+#include <linux/falloc.h>
#include <asm/ioctls.h>
@@ -346,6 +347,37 @@ EXPORT_SYMBOL(generic_block_fiemap);
#endif /* CONFIG_BLOCK */
+/*
+ * This provides compatibility with legacy XFS pre-allocation ioctls
+ * which predate the fallocate syscall.
+ *
+ * Only the l_start, l_len and l_whence fields of the 'struct space_resv'
+ * are used here, rest are ignored.
+ */
+static int ioctl_preallocate(struct file *filp, unsigned long arg)
+{
+ struct inode *inode = filp->f_path.dentry->d_inode;
+ struct space_resv sr;
+
+ if (copy_from_user(&sr, (struct space_resv __user *) arg, sizeof(sr)))
+ return -EFAULT;
+
+ switch (sr.l_whence) {
+ case SEEK_SET:
+ break;
+ case SEEK_CUR:
+ sr.l_start += filp->f_pos;
+ break;
+ case SEEK_END:
+ sr.l_start += i_size_read(inode);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start, sr.l_len);
+}
+
static int file_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
{
@@ -361,6 +393,11 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
return put_user(inode->i_sb->s_blocksize, p);
case FIONREAD:
return put_user(i_size_read(inode) - filp->f_pos, p);
+ case F_IOC_RESVSP:
+ case F_IOC_RESVSP64:
+ case F_IOC_UNRESVSP:
+ case F_IOC_UNRESVSP64:
+ return ioctl_preallocate(filp, arg);
}
return vfs_ioctl(filp, cmd, arg);
diff --git a/fs/open.c b/fs/open.c
index 83cdb9d..0703bcb 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -360,62 +360,61 @@ asmlinkage long sys_ftruncate64(unsigned int fd, loff_t length)
}
#endif
-asmlinkage long sys_fallocate(int fd, int mode, loff_t offset, loff_t len)
+long do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
{
- struct file *file;
struct inode *inode;
- long ret = -EINVAL;
+ long ret;
if (offset < 0 || len <= 0)
- goto out;
+ return -EINVAL;
/* Return error if mode is not supported */
- ret = -EOPNOTSUPP;
if (mode && !(mode & FALLOC_FL_KEEP_SIZE))
- goto out;
+ return -EOPNOTSUPP;
- ret = -EBADF;
- file = fget(fd);
- if (!file)
- goto out;
- if (!(file->f_mode & FMODE_WRITE))
- goto out_fput;
+ if (!file || !(file->f_mode & FMODE_WRITE))
+ return -EBADF;
/*
* Revalidate the write permissions, in case security policy has
* changed since the files were opened.
*/
ret = security_file_permission(file, MAY_WRITE);
if (ret)
- goto out_fput;
+ return ret;
inode = file->f_path.dentry->d_inode;
-
- ret = -ESPIPE;
if (S_ISFIFO(inode->i_mode))
- goto out_fput;
+ return -ESPIPE;
- ret = -ENODEV;
/*
* Let individual file system decide if it supports preallocation
* for directories or not.
*/
if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
- goto out_fput;
+ return -ENODEV;
- ret = -EFBIG;
/* Check for wrap through zero too */
if (((offset + len) > inode->i_sb->s_maxbytes) || ((offset + len) < 0))
- goto out_fput;
+ return -EFBIG;
if (inode->i_op && inode->i_op->fallocate)
- ret = inode->i_op->fallocate(inode, mode, offset, len);
+ return inode->i_op->fallocate(inode, mode, offset, len);
else
- ret = -EOPNOTSUPP;
+ return -EOPNOTSUPP;
+}
-out_fput:
- fput(file);
-out:
- return ret;
+asmlinkage long sys_fallocate(int fd, int mode, loff_t offset, loff_t len)
+{
+ struct file *file;
+ int error = -EBADF;
+
+ file = fget(fd);
+ if (file) {
+ error = do_fallocate(file, mode, offset, len);
+ fput(file);
+ }
+
+ return error;
}
/*
diff --git a/include/linux/falloc.h b/include/linux/falloc.h
index 8e912ab..4f2a727 100644
--- a/include/linux/falloc.h
+++ b/include/linux/falloc.h
@@ -3,4 +3,23 @@
#define FALLOC_FL_KEEP_SIZE 0x01 /* default is extend size */
+/*
+ * Space reservation / allocation ioctls and argument structure
+ * are designed to be compatible with the legacy XFS ioctls.
+ */
+struct space_resv {
+ __s16 l_type;
+ __s16 l_whence;
+ __s64 l_start;
+ __s64 l_len; /* len == 0 means until end of file */
+ __s32 l_sysid;
+ __u32 l_pid;
+ __s32 l_pad[4]; /* reserve area */
+};
+
+#define F_IOC_RESVSP _IOW('X', 40, struct space_resv)
+#define F_IOC_UNRESVSP _IOW('X', 41, struct space_resv)
+#define F_IOC_RESVSP64 _IOW('X', 42, struct space_resv)
+#define F_IOC_UNRESVSP64 _IOW('X', 43, struct space_resv)
+
#endif /* _FALLOC_H_ */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 4a853ef..b1d8f12 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1671,6 +1671,8 @@ static inline int break_lease(struct inode *inode, unsigned int mode)
extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs,
struct file *filp);
+extern long do_fallocate(struct file *file, int mode, loff_t offset,
+ loff_t len);
extern long do_sys_open(int dfd, const char __user *filename, int flags,
int mode);
extern struct file *filp_open(const char *, int, int);
On Mon, Dec 15, 2008 at 01:34:24PM +0530, Ankit Jain wrote:
> This patch adds ioctls to vfs for compatibility with legacy XFS
> pre-allocation ioctls (XFS_IOC_*RESVP*). The implementation
> effectively invokes sys_fallocate for the new ioctls.
> Note: These legacy ioctls are also implemented by OCFS2.
>
> There are some things that I'm not sure about:
> 1. Should the struct space_resv be exposed to user-space? If not,
> then what would be the right place to put it? And the ioctl
> definitions?
Yes. As far as where to put it, I'm not sure. Maybe falloc.h?
> 2. Should the corresponding ioctls be removed from ocfs2?
Well, a small amount of the code in fs/ocfs2/ioctl.c can certainly go away.
Shouldn't we be talking about doing the same for xfs too?
> Signed-off-by: Ankit Jain <[email protected]>
> ---
> fs/ioctl.c | 37 +++++++++++++++++++++++++++
> fs/open.c | 51 ++++++++++++++++++-------------------
> include/linux/falloc.h | 19 ++++++++++++++
> include/linux/fs.h | 2 +
> 4 files changed, 83 insertions(+), 26 deletions(-)
>
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 43e8b2c..5e565c8 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -15,6 +15,7 @@
> #include <linux/uaccess.h>
> #include <linux/writeback.h>
> #include <linux/buffer_head.h>
> +#include <linux/falloc.h>
>
> #include <asm/ioctls.h>
>
> @@ -346,6 +347,37 @@ EXPORT_SYMBOL(generic_block_fiemap);
>
> #endif /* CONFIG_BLOCK */
>
> +/*
> + * This provides compatibility with legacy XFS pre-allocation ioctls
> + * which predate the fallocate syscall.
> + *
> + * Only the l_start, l_len and l_whence fields of the 'struct space_resv'
> + * are used here, rest are ignored.
> + */
> +static int ioctl_preallocate(struct file *filp, unsigned long arg)
> +{
> + struct inode *inode = filp->f_path.dentry->d_inode;
> + struct space_resv sr;
> +
> + if (copy_from_user(&sr, (struct space_resv __user *) arg, sizeof(sr)))
> + return -EFAULT;
> +
> + switch (sr.l_whence) {
> + case SEEK_SET:
> + break;
> + case SEEK_CUR:
> + sr.l_start += filp->f_pos;
> + break;
> + case SEEK_END:
> + sr.l_start += i_size_read(inode);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start, sr.l_len);
> +}
> +
> static int file_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg)
> {
> @@ -361,6 +393,11 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
> return put_user(inode->i_sb->s_blocksize, p);
> case FIONREAD:
> return put_user(i_size_read(inode) - filp->f_pos, p);
> + case F_IOC_RESVSP:
> + case F_IOC_RESVSP64:
> + case F_IOC_UNRESVSP:
> + case F_IOC_UNRESVSP64:
> + return ioctl_preallocate(filp, arg);
This patch is not implementing proper support for F_IOC_UNRESVSP and
F_IOC_UNRESVSP64, so why are you catching those here? To be more clear,
those are used for freeing space in a file ("puching holes"), which
fallocate is not set up to do right now.
--Mark
--
Mark Fasheh
Mark Fasheh wrote:
> On Mon, Dec 15, 2008 at 01:34:24PM +0530, Ankit Jain wrote:
>> This patch adds ioctls to vfs for compatibility with legacy XFS
>> pre-allocation ioctls (XFS_IOC_*RESVP*). The implementation
>> effectively invokes sys_fallocate for the new ioctls.
>> Note: These legacy ioctls are also implemented by OCFS2.
>>
>> There are some things that I'm not sure about:
>> 1. Should the struct space_resv be exposed to user-space? If not,
>> then what would be the right place to put it? And the ioctl
>> definitions?
>
> Yes. As far as where to put it, I'm not sure. Maybe falloc.h?
I'd sort of rather not; why should that legacy struct space_resv be
available in a header... I thought this was for people already using the
xfs ioctl, in which case they are already using the xfs header... and if
they want preallocation in any new work, they should use fallocate()
instead, yes?
I mean it wouldn't hurt anything but it sort of confuses the interface
story IMHO.
-Eric
On Wed, Dec 17, 2008 at 03:06:14PM -0600, Eric Sandeen wrote:
> >> There are some things that I'm not sure about:
> >> 1. Should the struct space_resv be exposed to user-space? If not,
> >> then what would be the right place to put it? And the ioctl
> >> definitions?
> >
> > Yes. As far as where to put it, I'm not sure. Maybe falloc.h?
>
> I'd sort of rather not; why should that legacy struct space_resv be
> available in a header... I thought this was for people already using the
> xfs ioctl, in which case they are already using the xfs header... and if
> they want preallocation in any new work, they should use fallocate()
> instead, yes?
Actually, yeah agreed - I take that back. Old users are already getting them
from file system headers, new ones should use fallocate. So there's no need
to expose the ioctls to userspace, at least not if all we're talking about
is the RESVP ioctls.
--Mark
--
Mark Fasheh
On Dec 15, 2008, at 2:04 AM, Ankit Jain wrote:
> This patch adds ioctls to vfs for compatibility with legacy XFS
> pre-allocation ioctls (XFS_IOC_*RESVP*). The implementation
> effectively invokes sys_fallocate for the new ioctls.
I don't think we can use sys_fallocate for XFS_IOC_UNRESVSP*
commands, which suppose to release currently allocated file
blocks.
See more comments below.
Felix
> Note: These legacy ioctls are also implemented by OCFS2.
>
> There are some things that I'm not sure about:
> 1. Should the struct space_resv be exposed to user-space? If not,
> then what would be the right place to put it? And the ioctl
> definitions?
> 2. Should the corresponding ioctls be removed from ocfs2?
>
> Signed-off-by: Ankit Jain <[email protected]>
> ---
> fs/ioctl.c | 37 +++++++++++++++++++++++++++
> fs/open.c | 51 ++++++++++++++++++-------------------
> include/linux/falloc.h | 19 ++++++++++++++
> include/linux/fs.h | 2 +
> 4 files changed, 83 insertions(+), 26 deletions(-)
>
> diff --git a/fs/ioctl.c b/fs/ioctl.c
> index 43e8b2c..5e565c8 100644
> --- a/fs/ioctl.c
> +++ b/fs/ioctl.c
> @@ -15,6 +15,7 @@
> #include <linux/uaccess.h>
> #include <linux/writeback.h>
> #include <linux/buffer_head.h>
> +#include <linux/falloc.h>
>
> #include <asm/ioctls.h>
>
> @@ -346,6 +347,37 @@ EXPORT_SYMBOL(generic_block_fiemap);
>
> #endif /* CONFIG_BLOCK */
>
> +/*
> + * This provides compatibility with legacy XFS pre-allocation ioctls
> + * which predate the fallocate syscall.
> + *
> + * Only the l_start, l_len and l_whence fields of the 'struct
> space_resv'
> + * are used here, rest are ignored.
> + */
> +static int ioctl_preallocate(struct file *filp, unsigned long arg)
> +{
> + struct inode *inode = filp->f_path.dentry->d_inode;
> + struct space_resv sr;
> +
> + if (copy_from_user(&sr, (struct space_resv __user *) arg, sizeof
> (sr)))
> + return -EFAULT;
> +
> + switch (sr.l_whence) {
> + case SEEK_SET:
> + break;
> + case SEEK_CUR:
> + sr.l_start += filp->f_pos;
> + break;
> + case SEEK_END:
> + sr.l_start += i_size_read(inode);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start,
> sr.l_len);
> +}
> +
> static int file_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg)
> {
> @@ -361,6 +393,11 @@ static int file_ioctl(struct file *filp,
> unsigned int cmd,
> return put_user(inode->i_sb->s_blocksize, p);
> case FIONREAD:
> return put_user(i_size_read(inode) - filp->f_pos, p);
> + case F_IOC_RESVSP:
> + case F_IOC_RESVSP64:
> + case F_IOC_UNRESVSP:
> + case F_IOC_UNRESVSP64:
> + return ioctl_preallocate(filp, arg);
At this point the original command 'cmd' is dropped,
and ioctl_preallocate() assumes F_IOC_RESVSP*.
Indeed, in the following path
ioctl_preallocate
do_fallocate
.fallocate/xfs_vn_fallocate
xfs_change_file_space(XFS_IOC_RESVSP)
UNRESVSP is never considered.
> }
>
> return vfs_ioctl(filp, cmd, arg);
> diff --git a/fs/open.c b/fs/open.c
> index 83cdb9d..0703bcb 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -360,62 +360,61 @@ asmlinkage long sys_ftruncate64(unsigned int
> fd, loff_t length)
> }
> #endif
>
> -asmlinkage long sys_fallocate(int fd, int mode, loff_t offset,
> loff_t len)
> +long do_fallocate(struct file *file, int mode, loff_t offset,
> loff_t len)
> {
> - struct file *file;
> struct inode *inode;
> - long ret = -EINVAL;
> + long ret;
>
> if (offset < 0 || len <= 0)
> - goto out;
> + return -EINVAL;
>
> /* Return error if mode is not supported */
> - ret = -EOPNOTSUPP;
> if (mode && !(mode & FALLOC_FL_KEEP_SIZE))
> - goto out;
> + return -EOPNOTSUPP;
>
> - ret = -EBADF;
> - file = fget(fd);
> - if (!file)
> - goto out;
> - if (!(file->f_mode & FMODE_WRITE))
> - goto out_fput;
> + if (!file || !(file->f_mode & FMODE_WRITE))
> + return -EBADF;
> /*
> * Revalidate the write permissions, in case security policy has
> * changed since the files were opened.
> */
> ret = security_file_permission(file, MAY_WRITE);
> if (ret)
> - goto out_fput;
> + return ret;
>
> inode = file->f_path.dentry->d_inode;
> -
> - ret = -ESPIPE;
> if (S_ISFIFO(inode->i_mode))
> - goto out_fput;
> + return -ESPIPE;
>
> - ret = -ENODEV;
> /*
> * Let individual file system decide if it supports preallocation
> * for directories or not.
> */
> if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
> - goto out_fput;
> + return -ENODEV;
>
> - ret = -EFBIG;
> /* Check for wrap through zero too */
> if (((offset + len) > inode->i_sb->s_maxbytes) || ((offset + len)
> < 0))
> - goto out_fput;
> + return -EFBIG;
>
> if (inode->i_op && inode->i_op->fallocate)
> - ret = inode->i_op->fallocate(inode, mode, offset, len);
> + return inode->i_op->fallocate(inode, mode, offset, len);
> else
> - ret = -EOPNOTSUPP;
> + return -EOPNOTSUPP;
> +}
>
> -out_fput:
> - fput(file);
> -out:
> - return ret;
> +asmlinkage long sys_fallocate(int fd, int mode, loff_t offset,
> loff_t len)
> +{
> + struct file *file;
> + int error = -EBADF;
> +
> + file = fget(fd);
> + if (file) {
> + error = do_fallocate(file, mode, offset, len);
> + fput(file);
> + }
> +
> + return error;
> }
>
> /*
> diff --git a/include/linux/falloc.h b/include/linux/falloc.h
> index 8e912ab..4f2a727 100644
> --- a/include/linux/falloc.h
> +++ b/include/linux/falloc.h
> @@ -3,4 +3,23 @@
>
> #define FALLOC_FL_KEEP_SIZE 0x01 /* default is extend size */
>
> +/*
> + * Space reservation / allocation ioctls and argument structure
> + * are designed to be compatible with the legacy XFS ioctls.
> + */
> +struct space_resv {
> + __s16 l_type;
> + __s16 l_whence;
> + __s64 l_start;
> + __s64 l_len; /* len == 0 means until end of file */
> + __s32 l_sysid;
> + __u32 l_pid;
> + __s32 l_pad[4]; /* reserve area */
> +};
> +
> +#define F_IOC_RESVSP _IOW('X', 40, struct space_resv)
> +#define F_IOC_UNRESVSP _IOW('X', 41, struct space_resv)
> +#define F_IOC_RESVSP64 _IOW('X', 42, struct space_resv)
> +#define F_IOC_UNRESVSP64 _IOW('X', 43, struct space_resv)
> +
> #endif /* _FALLOC_H_ */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 4a853ef..b1d8f12 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1671,6 +1671,8 @@ static inline int break_lease(struct inode
> *inode, unsigned int mode)
>
> extern int do_truncate(struct dentry *, loff_t start, unsigned int
> time_attrs,
> struct file *filp);
> +extern long do_fallocate(struct file *file, int mode, loff_t offset,
> + loff_t len);
> extern long do_sys_open(int dfd, const char __user *filename, int
> flags,
> int mode);
> extern struct file *filp_open(const char *, int, int);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-
> fsdevel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Mark Fasheh wrote:
>> 2. Should the corresponding ioctls be removed from ocfs2?
>
> Well, a small amount of the code in fs/ocfs2/ioctl.c can certainly go away.
> Shouldn't we be talking about doing the same for xfs too?
Yep. I'll cook up cleanup patches for these two and post them.
-Ankit
>
>
>> Signed-off-by: Ankit Jain <[email protected]>
>> ---
>> fs/ioctl.c | 37 +++++++++++++++++++++++++++
>> fs/open.c | 51 ++++++++++++++++++-------------------
>> include/linux/falloc.h | 19 ++++++++++++++
>> include/linux/fs.h | 2 +
>> 4 files changed, 83 insertions(+), 26 deletions(-)
>>
>> diff --git a/fs/ioctl.c b/fs/ioctl.c
>> index 43e8b2c..5e565c8 100644
>> --- a/fs/ioctl.c
>> +++ b/fs/ioctl.c
>> @@ -15,6 +15,7 @@
>> #include <linux/uaccess.h>
>> #include <linux/writeback.h>
>> #include <linux/buffer_head.h>
>> +#include <linux/falloc.h>
>>
>> #include <asm/ioctls.h>
>>
>> @@ -346,6 +347,37 @@ EXPORT_SYMBOL(generic_block_fiemap);
>>
>> #endif /* CONFIG_BLOCK */
>>
>> +/*
>> + * This provides compatibility with legacy XFS pre-allocation ioctls
>> + * which predate the fallocate syscall.
>> + *
>> + * Only the l_start, l_len and l_whence fields of the 'struct space_resv'
>> + * are used here, rest are ignored.
>> + */
>> +static int ioctl_preallocate(struct file *filp, unsigned long arg)
>> +{
>> + struct inode *inode = filp->f_path.dentry->d_inode;
>> + struct space_resv sr;
>> +
>> + if (copy_from_user(&sr, (struct space_resv __user *) arg, sizeof(sr)))
>> + return -EFAULT;
>> +
>> + switch (sr.l_whence) {
>> + case SEEK_SET:
>> + break;
>> + case SEEK_CUR:
>> + sr.l_start += filp->f_pos;
>> + break;
>> + case SEEK_END:
>> + sr.l_start += i_size_read(inode);
>> + break;
>> + default:
>> + return -EINVAL;
>> + }
>> +
>> + return do_fallocate(filp, FALLOC_FL_KEEP_SIZE, sr.l_start, sr.l_len);
>> +}
>> +
>> static int file_ioctl(struct file *filp, unsigned int cmd,
>> unsigned long arg)
>> {
>> @@ -361,6 +393,11 @@ static int file_ioctl(struct file *filp, unsigned int cmd,
>> return put_user(inode->i_sb->s_blocksize, p);
>> case FIONREAD:
>> return put_user(i_size_read(inode) - filp->f_pos, p);
>> + case F_IOC_RESVSP:
>> + case F_IOC_RESVSP64:
>> + case F_IOC_UNRESVSP:
>> + case F_IOC_UNRESVSP64:
>> + return ioctl_preallocate(filp, arg);
>
> This patch is not implementing proper support for F_IOC_UNRESVSP and
> F_IOC_UNRESVSP64, so why are you catching those here? To be more clear,
> those are used for freeing space in a file ("puching holes"), which
> fallocate is not set up to do right now.
> --Mark
>
> --
> Mark Fasheh
Mark Fasheh wrote:
> On Wed, Dec 17, 2008 at 03:06:14PM -0600, Eric Sandeen wrote:
>
> Actually, yeah agreed - I take that back. Old users are already getting them
> from file system headers, new ones should use fallocate. So there's no need
> to expose the ioctls to userspace, at least not if all we're talking about
> is the RESVP ioctls.
Makes sense. So I can just keep them in falloc.h in a #ifdef __KERNEL__ ? Or
should this be in a new ioctl.h ?
-Ankit
> --Mark
>
> --
> Mark Fasheh
Felix Blyakher wrote:
>
>> This patch adds ioctls to vfs for compatibility with legacy XFS
>> pre-allocation ioctls (XFS_IOC_*RESVP*). The implementation
>> effectively invokes sys_fallocate for the new ioctls.
>
> I don't think we can use sys_fallocate for XFS_IOC_UNRESVSP*
> commands, which suppose to release currently allocated file
> blocks.
> See more comments below.
My bad :( I'll drop the *UNRESV* completely from the new ioctl and
post an updated patch.
-Ankit
Mark Fasheh wrote:
> On Mon, Dec 15, 2008 at 01:34:24PM +0530, Ankit Jain wrote:
>> 2. Should the corresponding ioctls be removed from ocfs2?
>
> Well, a small amount of the code in fs/ocfs2/ioctl.c can certainly go away.
> Shouldn't we be talking about doing the same for xfs too?
Reading the code a bit, my understanding is that as compat_ioctl is also
supported and that just delegates to ioctl (ocfs2_ioctl), so we can't
remove the *_RESVSP* handling.
Same goes for xfs also.
Does that sound fair or did I not understand it correctly?
-Ankit