[PATCH 1/2] xfs_spaceman: XFS_IOC_FIEMAPFS ioctl replacing previous FS_IOC_FIEMAPFS

[PATCH 1/2] xfs_spaceman: XFS_IOC_FIEMAPFS ioctl replacing previous FS_IOC_FIEMAPFS

[Dhruvesh Rathore]

The original kernel patch that Dave wrote for xfs_spaceman implemented
FS_IOC_FIEMAPFS ioctl, here is the original fiemap extension patch:

http://oss.sgi.com/archives/xfs/2012-10/msg00363.html

These patches were not posted and were just forward ported to a current 
3.18-rc7+ for-next XFS tree and were pushed to the fiemapfs branch in Dave's
kernel tree at:

git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git

The original xfs_spaceman tool that Dave wrote to call the fiemap
interface and make use of it is here:

http://oss.sgi.com/archives/xfs/2012-10/msg00366.html

Dave just updated it to the 3.2.2 code base and pushed it to the
spaceman branch in this tree:

git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git

This patch is concerned with turning FS_IOC_FIEMAPFS, present in the earlier
version of xfs_spaceman into an XFS specific ioctl called XFS_IOC_FIEMAPFS
that uses the fiemap plumbing. Please do comment. 

Signed-off-by: 
Dhruvesh Rathore <dhruvesh_r@outlook.com>
Amey Ruikar <ameyruikar@yahoo.com>
Somdeep Dey <somdeepdey10@gmail.com>
---
linux-xfs/fs/xfs/xfs_fs.h		| 1 +
linux-xfs/fs/xfs/xfs_ioctl.c		| 71 ++++-
xfsprogs-3.2.2/include/xfs_fs.h		| 1 +
xfsprogs-3.2.2/spaceman/freesp.c	| 10 ++++++---
4 files changed, 79 insertions(+), 4 deletions(-) 

---------------------------------------------------------------------------------------

--- a/linux-xfs/fs/xfs/xfs_fs.h	2015-01-04 15:26:46.954401773 +0530
+++ b/linux-xfs/fs/xfs/xfs_fs.h	2015-01-04 11:29:54.531652554 +0530
@@ -505,6 +505,7 @@
 #define XFS_IOC_DIOINFO	_IOR ('X', 30, struct dioattr)
 #define XFS_IOC_FSGETXATTR	_IOR ('X', 31, struct fsxattr)
 #define XFS_IOC_FSSETXATTR	_IOW ('X', 32, struct fsxattr)
+#define XFS_IOC_FIEMAPFS	_IOWR('X', 33, struct fiemap)
 #define XFS_IOC_ALLOCSP64	_IOW ('X', 36, struct xfs_flock64)
 #define XFS_IOC_FREESP64	_IOW ('X', 37, struct xfs_flock64)
 #define XFS_IOC_GETBMAP	_IOWR('X', 38, struct getbmap)

----------------------------------------------------------------------------------------

--- a/linux-xfs/fs/xfs/xfs_ioctl.c	2015-01-04 15:45:26.518359725 +0530
+++ b/linux-xfs/fs/xfs/xfs_ioctl.c	2015-01-04 15:24:06.030407817 +0530
@@ -49,6 +49,8 @@
 #include <linux/exportfs.h>
 
 
+/* So that the fiemap access checks can't overflow on 32 bit machines. */   		 
+#define FIEMAP_MAX_EXTENTS	(UINT_MAX / sizeof(struct fiemap_extent))		 
 
 /*
  * xfs_find_handle maps from userspace xfs_fsop_handlereq structure to
@@ -1511,8 +1513,73 @@
 	return error;
 }
 
+static int fiemap_check_ranges(struct super_block *sb,
+			       u64 start, u64 len, u64 *new_len)
+{
+	u64 maxbytes = (u64) sb->s_maxbytes;
+
+	*new_len = len;
+
+	if (len == 0)
+		return -EINVAL;
+
+	if (start > maxbytes)
+		return -EFBIG;
+
+	/*
+	 * Shrink request scope to what the fs can actually handle.
+	 */
+	if (len > maxbytes || (maxbytes - len) < start)
+		*new_len = maxbytes - start;
 
+	return 0;
+}
+
+static int xfsctl_fiemapfs(struct super_block *sb, unsigned long arg)				
+{
+	struct fiemap fiemap;									
+	struct fiemap __user *ufiemap = (struct fiemap __user *) arg;
+	struct fiemap_extent_info fieinfo = { 0, };
+	u64 len;
+	int error;
+
+	if (!sb->s_op->fiemapfs)
+		return -EOPNOTSUPP;
 
+	if (copy_from_user(&fiemap, ufiemap, sizeof(fiemap)))
+		return -EFAULT;
+
+	if (fiemap.fm_extent_count > FIEMAP_MAX_EXTENTS)
+		return -EINVAL;
+
+	error = fiemap_check_ranges(sb, fiemap.fm_start, fiemap.fm_length,
+				    &len);
+	if (error)
+		return error;
+
+	fieinfo.fi_flags = fiemap.fm_flags;
+	fieinfo.fi_extents_max = fiemap.fm_extent_count;
+	fieinfo.fi_extents_start = ufiemap->fm_extents;
+
+	if (fiemap.fm_extent_count != 0 &&
+	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
+		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
+		return -EFAULT;
+
+	if (fiemap.fm_extent_count != 0 &&
+	    (fiemap.fm_flags & FIEMAPFS_FLAG_FREESP_SIZE_HINT) &&
+	    !access_ok(VERIFY_READ, fieinfo.fi_extents_start,
+		       sizeof(struct fiemap_extent)))
+		return -EFAULT;
+
+	error = sb->s_op->fiemapfs(sb, &fieinfo, fiemap.fm_start, len);
+	fiemap.fm_flags = fieinfo.fi_flags;
+	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
+	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
+		error = -EFAULT;
+
+	return error;
+}
 
 /*
  * Note: some of the ioctl's return positive numbers as a
@@ -1570,7 +1637,9 @@
 		return 0;
 	}
 
-	
+	case XFS_IOC_FIEMAPFS:						
+	   return xfsctl_fiemapfs(inode->i_sb, p);			
+
 	case XFS_IOC_FSBULKSTAT_SINGLE:
 	case XFS_IOC_FSBULKSTAT:
 	case XFS_IOC_FSINUMBERS:

--------------------------------------------------------------------------------------------

--- a/xfsprogs-3.2.2/include/xfs_fs.h	2015-01-04 15:26:50.954401773 +0530
+++ b/xfsprogs-3.2.2/include/xfs_fs.h	2015-01-04 11:29:59.531652554 +0530
@@ -505,6 +505,7 @@
 #define XFS_IOC_DIOINFO	_IOR ('X', 30, struct dioattr)
 #define XFS_IOC_FSGETXATTR	_IOR ('X', 31, struct fsxattr)
 #define XFS_IOC_FSSETXATTR	_IOW ('X', 32, struct fsxattr)
+#define XFS_IOC_FIEMAPFS	_IOWR('X', 33, struct fiemap)
 #define XFS_IOC_ALLOCSP64	_IOW ('X', 36, struct xfs_flock64)
 #define XFS_IOC_FREESP64	_IOW ('X', 37, struct xfs_flock64)
 #define XFS_IOC_GETBMAP	_IOWR('X', 38, struct getbmap)

--------------------------------------------------------------------------------------------

--- a/xfsprogs-3.2.2/spaceman/freesp.c	2015-01-04 15:39:58.446372047 +0530
+++ b/xfsprogs-3.2.2/spaceman/freesp.c	2015-01-04 15:38:30.294375357 +0530
@@ -31,7 +31,7 @@
 #define FIEMAPFS_FLAG_FREESP_SIZE_HINT	0x20000000
 #define	FIEMAPFS_FLAG_FREESP_CONTINUE   0x10000000
 
-#define FS_IOC_FIEMAPFS			_IOWR('f', 12, struct fiemap)
+#define XFS_IOC_FIEMAPFS			_IOWR('X', 33, struct fiemap)
 #endif
 
 typedef struct histent
@@ -201,9 +201,9 @@
 		fiemap->fm_length = length;
 		fiemap->fm_extent_count = NR_EXTENTS;
 
-		ret = ioctl(file->fd, XFS_IOC_FIEMAPFS, (unsigned long)fiemap);
+		ret = xfsctl(file->name,file->fd, XFS_IOC_FIEMAPFS, (unsigned long)fiemap);
 		if (ret < 0) {	
-			fprintf(stderr, "%s: ioctl(FS_IOC_FIEMAPFS) [\"%s\"]: "	
+			fprintf(stderr, "%s: xfsctl(XFS_IOC_FIEMAPFS) [\"%s\"]: "	
 				"%s\n", progname, file->name, strerror(errno));
 			free(fiemap);
 			exitcode = 1;
@@ -338,6 +338,10 @@
 
 	if (!init(argc, argv))
 		return 0;
+
+	if (dumpflag)
+		printf("%8s %8s %8s\n", "agno", "agbno", "len");	
+
 	for (agno = 0; agno < file->geom.agcount; agno++)  {
 		if (inaglist(agno))
 			scan_ag(agno);

----------------------------------------------------------------------------------------------

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH 1/2] xfs_spaceman: XFS_IOC_FIEMAPFS ioctl replacing previous FS_IOC_FIEMAPFS

[Dhruvesh Rathore]

The original kernel patch that Dave wrote for xfs_spaceman implemented
FS_IOC_FIEMAPFS ioctl, here is the original fiemap extension patch:

http://oss.sgi.com/archives/xfs/2012-10/msg00363.html

These patches were not posted and were just forward ported to a current 
3.18-rc7+ for-next XFS tree and were pushed to the fiemapfs branch in Dave's
kernel tree at:

git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git

The original xfs_spaceman tool that Dave wrote to call the fiemap
interface and make use of it is here:

http://oss.sgi.com/archives/xfs/2012-10/msg00366.html

Dave just updated it to the 3.2.2 code base and pushed it to the
spaceman branch in this tree:

git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git

This patch is concerned with turning FS_IOC_FIEMAPFS, present in the earlier
version of xfs_spaceman into an XFS specific ioctl called XFS_IOC_FIEMAPFS
that uses the fiemap plumbing. Please do comment. 

Signed-off-by: 
Dhruvesh Rathore <dhruvesh_r@outlook.com>
Amey Ruikar <ameyruikar@yahoo.com>
Somdeep Dey <somdeepdey10@gmail.com>
---
linux-xfs/fs/xfs/xfs_fs.h		| 1 +
linux-xfs/fs/xfs/xfs_ioctl.c		| 71 ++++-
xfsprogs-3.2.2/include/xfs_fs.h		| 1 +
xfsprogs-3.2.2/spaceman/freesp.c	| 10 ++++++---
4 files changed, 79 insertions(+), 4 deletions(-) 

---------------------------------------------------------------------------------------

--- a/linux-xfs/fs/xfs/xfs_fs.h	2015-01-04 15:26:46.954401773 +0530
+++ b/linux-xfs/fs/xfs/xfs_fs.h	2015-01-04 11:29:54.531652554 +0530
@@ -505,6 +505,7 @@
 #define XFS_IOC_DIOINFO	_IOR ('X', 30, struct dioattr)
 #define XFS_IOC_FSGETXATTR	_IOR ('X', 31, struct fsxattr)
 #define XFS_IOC_FSSETXATTR	_IOW ('X', 32, struct fsxattr)
+#define XFS_IOC_FIEMAPFS	_IOWR('X', 33, struct fiemap)
 #define XFS_IOC_ALLOCSP64	_IOW ('X', 36, struct xfs_flock64)
 #define XFS_IOC_FREESP64	_IOW ('X', 37, struct xfs_flock64)
 #define XFS_IOC_GETBMAP	_IOWR('X', 38, struct getbmap)

----------------------------------------------------------------------------------------

--- a/linux-xfs/fs/xfs/xfs_ioctl.c	2015-01-04 15:45:26.518359725 +0530
+++ b/linux-xfs/fs/xfs/xfs_ioctl.c	2015-01-04 15:24:06.030407817 +0530
@@ -49,6 +49,8 @@
 #include <linux/exportfs.h>
 
 
+/* So that the fiemap access checks can't overflow on 32 bit machines. */   		 
+#define FIEMAP_MAX_EXTENTS	(UINT_MAX / sizeof(struct fiemap_extent))		 
 
 /*
  * xfs_find_handle maps from userspace xfs_fsop_handlereq structure to
@@ -1511,8 +1513,73 @@
 	return error;
 }
 
+static int fiemap_check_ranges(struct super_block *sb,
+			       u64 start, u64 len, u64 *new_len)
+{
+	u64 maxbytes = (u64) sb->s_maxbytes;
+
+	*new_len = len;
+
+	if (len == 0)
+		return -EINVAL;
+
+	if (start > maxbytes)
+		return -EFBIG;
+
+	/*
+	 * Shrink request scope to what the fs can actually handle.
+	 */
+	if (len > maxbytes || (maxbytes - len) < start)
+		*new_len = maxbytes - start;
 
+	return 0;
+}
+
+static int xfsctl_fiemapfs(struct super_block *sb, unsigned long arg)				
+{
+	struct fiemap fiemap;									
+	struct fiemap __user *ufiemap = (struct fiemap __user *) arg;
+	struct fiemap_extent_info fieinfo = { 0, };
+	u64 len;
+	int error;
+
+	if (!sb->s_op->fiemapfs)
+		return -EOPNOTSUPP;
 
+	if (copy_from_user(&fiemap, ufiemap, sizeof(fiemap)))
+		return -EFAULT;
+
+	if (fiemap.fm_extent_count > FIEMAP_MAX_EXTENTS)
+		return -EINVAL;
+
+	error = fiemap_check_ranges(sb, fiemap.fm_start, fiemap.fm_length,
+				    &len);
+	if (error)
+		return error;
+
+	fieinfo.fi_flags = fiemap.fm_flags;
+	fieinfo.fi_extents_max = fiemap.fm_extent_count;
+	fieinfo.fi_extents_start = ufiemap->fm_extents;
+
+	if (fiemap.fm_extent_count != 0 &&
+	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
+		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
+		return -EFAULT;
+
+	if (fiemap.fm_extent_count != 0 &&
+	    (fiemap.fm_flags & FIEMAPFS_FLAG_FREESP_SIZE_HINT) &&
+	    !access_ok(VERIFY_READ, fieinfo.fi_extents_start,
+		       sizeof(struct fiemap_extent)))
+		return -EFAULT;
+
+	error = sb->s_op->fiemapfs(sb, &fieinfo, fiemap.fm_start, len);
+	fiemap.fm_flags = fieinfo.fi_flags;
+	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
+	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
+		error = -EFAULT;
+
+	return error;
+}
 
 /*
  * Note: some of the ioctl's return positive numbers as a
@@ -1570,7 +1637,9 @@
 		return 0;
 	}
 
-	
+	case XFS_IOC_FIEMAPFS:						
+	   return xfsctl_fiemapfs(inode->i_sb, p);			
+
 	case XFS_IOC_FSBULKSTAT_SINGLE:
 	case XFS_IOC_FSBULKSTAT:
 	case XFS_IOC_FSINUMBERS:

--------------------------------------------------------------------------------------------

--- a/xfsprogs-3.2.2/include/xfs_fs.h	2015-01-04 15:26:50.954401773 +0530
+++ b/xfsprogs-3.2.2/include/xfs_fs.h	2015-01-04 11:29:59.531652554 +0530
@@ -505,6 +505,7 @@
 #define XFS_IOC_DIOINFO	_IOR ('X', 30, struct dioattr)
 #define XFS_IOC_FSGETXATTR	_IOR ('X', 31, struct fsxattr)
 #define XFS_IOC_FSSETXATTR	_IOW ('X', 32, struct fsxattr)
+#define XFS_IOC_FIEMAPFS	_IOWR('X', 33, struct fiemap)
 #define XFS_IOC_ALLOCSP64	_IOW ('X', 36, struct xfs_flock64)
 #define XFS_IOC_FREESP64	_IOW ('X', 37, struct xfs_flock64)
 #define XFS_IOC_GETBMAP	_IOWR('X', 38, struct getbmap)

--------------------------------------------------------------------------------------------

--- a/xfsprogs-3.2.2/spaceman/freesp.c	2015-01-04 15:39:58.446372047 +0530
+++ b/xfsprogs-3.2.2/spaceman/freesp.c	2015-01-04 15:38:30.294375357 +0530
@@ -31,7 +31,7 @@
 #define FIEMAPFS_FLAG_FREESP_SIZE_HINT	0x20000000
 #define	FIEMAPFS_FLAG_FREESP_CONTINUE   0x10000000
 
-#define FS_IOC_FIEMAPFS			_IOWR('f', 12, struct fiemap)
+#define XFS_IOC_FIEMAPFS			_IOWR('X', 33, struct fiemap)
 #endif
 
 typedef struct histent
@@ -201,9 +201,9 @@
 		fiemap->fm_length = length;
 		fiemap->fm_extent_count = NR_EXTENTS;
 
-		ret = ioctl(file->fd, XFS_IOC_FIEMAPFS, (unsigned long)fiemap);
+		ret = xfsctl(file->name,file->fd, XFS_IOC_FIEMAPFS, (unsigned long)fiemap);
 		if (ret < 0) {	
-			fprintf(stderr, "%s: ioctl(FS_IOC_FIEMAPFS) [\"%s\"]: "	
+			fprintf(stderr, "%s: xfsctl(XFS_IOC_FIEMAPFS) [\"%s\"]: "	
 				"%s\n", progname, file->name, strerror(errno));
 			free(fiemap);
 			exitcode = 1;
@@ -338,6 +338,10 @@
 
 	if (!init(argc, argv))
 		return 0;
+
+	if (dumpflag)
+		printf("%8s %8s %8s\n", "agno", "agbno", "len");	
+
 	for (agno = 0; agno < file->geom.agcount; agno++)  {
 		if (inaglist(agno))
 			scan_ag(agno);

----------------------------------------------------------------------------------------------

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs_spaceman: XFS_IOC_FIEMAPFS ioctl replacing previous FS_IOC_FIEMAPFS

[Dave Chinner]

On Fri, Jan 23, 2015 at 09:03:44AM +0530, Dhruvesh Rathore wrote:
> The original kernel patch that Dave wrote for xfs_spaceman implemented
> FS_IOC_FIEMAPFS ioctl, here is the original fiemap extension patch:
> 
> http://oss.sgi.com/archives/xfs/2012-10/msg00363.html
> 
> These patches were not posted and were just forward ported to a current 
> 3.18-rc7+ for-next XFS tree and were pushed to the fiemapfs branch in Dave's
> kernel tree at:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/dgc/linux-xfs.git
> 
> The original xfs_spaceman tool that Dave wrote to call the fiemap
> interface and make use of it is here:
> 
> http://oss.sgi.com/archives/xfs/2012-10/msg00366.html
> 
> Dave just updated it to the 3.2.2 code base and pushed it to the
> spaceman branch in this tree:
> 
> git://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git
> 
> This patch is concerned with turning FS_IOC_FIEMAPFS, present in the earlier
> version of xfs_spaceman into an XFS specific ioctl called XFS_IOC_FIEMAPFS
> that uses the fiemap plumbing. Please do comment. 

So this commentary belongs in a patch series description, usually
titled "[PATCH 0/X] <patch series title>".

Then the patch itself will have a title/ description along the lines
of:

[PATCH 1/X] xfs: add XFS_IOC_FIEMAPFS

The FS_IOC_FIEMAPFS ioctl is not going to be merged, so convert the
existing code to use an XFS specific ioctl named XFS_IOC_FIEMAPFS
and plumb the code to use this ioctl.

> Signed-off-by: 
> Dhruvesh Rathore <dhruvesh_r@outlook.com>
> Amey Ruikar <ameyruikar@yahoo.com>
> Somdeep Dey <somdeepdey10@gmail.com>

One name per s-o-b. i.e.

Signed-off-by: Dhruvesh Rathore <dhruvesh_r@outlook.com>
Signed-off-by: Amey Ruikar <ameyruikar@yahoo.com>
Signed-off-by: Somdeep Dey <somdeepdey10@gmail.com>

> ---
> linux-xfs/fs/xfs/xfs_fs.h		| 1 +
> linux-xfs/fs/xfs/xfs_ioctl.c		| 71 ++++-
> xfsprogs-3.2.2/include/xfs_fs.h		| 1 +
> xfsprogs-3.2.2/spaceman/freesp.c	| 10 ++++++---
> 4 files changed, 79 insertions(+), 4 deletions(-) 

This needs to be separated into two patches - one for the kernel
changes, and one for the userspace changes. What I'd suggest that
you do is get the changes into a git tree with the correct commit
message that you want, then use git-send-email to send the commit.
That way you'll be sending patches that correctly formatted and
separated by tree.

> --- a/linux-xfs/fs/xfs/xfs_ioctl.c	2015-01-04 15:45:26.518359725 +0530
> +++ b/linux-xfs/fs/xfs/xfs_ioctl.c	2015-01-04 15:24:06.030407817 +0530
> @@ -49,6 +49,8 @@
>  #include <linux/exportfs.h>
>  
>  
> +/* So that the fiemap access checks can't overflow on 32 bit machines. */   		 
> +#define FIEMAP_MAX_EXTENTS	(UINT_MAX / sizeof(struct fiemap_extent))		 

Just move to include/linux/fs.h where the FS_IOC_FIEMAP definitions
are.

Also, there is trailing whitespace on the lines. I'd suggest that
you might like to use scripts/checkpatch.pl to find these simple
whitespace issues. if you use vim, then add this to your local
.vimrc file and it will highlight trailing whitespace in red:

" highlight whitespace damage
highlight RedundantSpaces ctermbg=red guibg=red
match RedundantSpaces /\s\+$\| \+\ze\t/

> +static int fiemap_check_ranges(struct super_block *sb,
> +			       u64 start, u64 len, u64 *new_len)
> +{
> +	u64 maxbytes = (u64) sb->s_maxbytes;
> +
> +	*new_len = len;
> +
> +	if (len == 0)
> +		return -EINVAL;
> +
> +	if (start > maxbytes)
> +		return -EFBIG;
> +
> +	/*
> +	 * Shrink request scope to what the fs can actually handle.
> +	 */
> +	if (len > maxbytes || (maxbytes - len) < start)
> +		*new_len = maxbytes - start;
>  
> +	return 0;
> +}

Rather than copying this code, I'd suggest that you remove the "static"
declaration, export the symbol and add a prototype to
include/linux/fs.h similar to fiemap_check_flags().

> +static int xfsctl_fiemapfs(struct super_block *sb, unsigned long arg)				

- more trailing whitespace.
- xfs_ioctl_fiemapfs() follows the naming convention used in the
  rest of the file
- this is an XFS specific function now, so should pass the xfs_mount
  rather than the VFS super_block.
- need a comment saying it was mostly copied from fs/ioctl.c::ioctl_fiemap().
- arg has already marked as __user converted in the calling
  function, so we need to retain the __user annotations here.
- format for XFS functions is this:

/*
 * Mostly copied from ioctl_fiemap()
 */
static int
xfs_ioctl_fiemapfs(
	struct xfs_mount	*mp,
	void __user		*arg)


> +{
> +	struct fiemap fiemap;									
> +	struct fiemap __user *ufiemap = (struct fiemap __user *) arg;
> +	struct fiemap_extent_info fieinfo = { 0, };
> +	u64 len;
> +	int error;

formatting of declarations is usually aligned to that of the
function declaration. i.e:

	struct fiemap		fiemap;
	struct fiemap __user	*ufiemap = (struct fiemap __user *) arg;
	struct fiemap_extent_info fieinfo = { 0, };
	u64			len;
	int			error;

> +	if (!sb->s_op->fiemapfs)
> +		return -EOPNOTSUPP;

No need to check this - we can call the XFS function directly.

>  
> +	if (copy_from_user(&fiemap, ufiemap, sizeof(fiemap)))
> +		return -EFAULT;
> +
> +	if (fiemap.fm_extent_count > FIEMAP_MAX_EXTENTS)
> +		return -EINVAL;
> +
> +	error = fiemap_check_ranges(sb, fiemap.fm_start, fiemap.fm_length,
> +				    &len);
> +	if (error)
> +		return error;
> +
> +	fieinfo.fi_flags = fiemap.fm_flags;
> +	fieinfo.fi_extents_max = fiemap.fm_extent_count;
> +	fieinfo.fi_extents_start = ufiemap->fm_extents;
> +
> +	if (fiemap.fm_extent_count != 0 &&
> +	    !access_ok(VERIFY_WRITE, fieinfo.fi_extents_start,
> +		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
> +		return -EFAULT;
> +
> +	if (fiemap.fm_extent_count != 0 &&
> +	    (fiemap.fm_flags & FIEMAPFS_FLAG_FREESP_SIZE_HINT) &&
> +	    !access_ok(VERIFY_READ, fieinfo.fi_extents_start,
> +		       sizeof(struct fiemap_extent)))
> +		return -EFAULT;
> +
> +	error = sb->s_op->fiemapfs(sb, &fieinfo, fiemap.fm_start, len);

Call the xfs function directly.

And, in doing so, this will mean you can remove the ->fiemapfs
definitions from include/linux/fs.h

> +	fiemap.fm_flags = fieinfo.fi_flags;
> +	fiemap.fm_mapped_extents = fieinfo.fi_extents_mapped;
> +	if (copy_to_user(ufiemap, &fiemap, sizeof(fiemap)))
> +		error = -EFAULT;
> +
> +	return error;
> +}
>  
>  /*
>   * Note: some of the ioctl's return positive numbers as a
> @@ -1570,7 +1637,9 @@
>  		return 0;
>  	}
>  
> -	
> +	case XFS_IOC_FIEMAPFS:						
> +	   return xfsctl_fiemapfs(inode->i_sb, p);			

8 space tabs and lots of trailing whitespace.

		return xfs_ioctl_fiemapfs(mp, arg);

> --- a/xfsprogs-3.2.2/spaceman/freesp.c	2015-01-04 15:39:58.446372047 +0530
> +++ b/xfsprogs-3.2.2/spaceman/freesp.c	2015-01-04 15:38:30.294375357 +0530
> @@ -31,7 +31,7 @@
>  #define FIEMAPFS_FLAG_FREESP_SIZE_HINT	0x20000000
>  #define	FIEMAPFS_FLAG_FREESP_CONTINUE   0x10000000
>  
> -#define FS_IOC_FIEMAPFS			_IOWR('f', 12, struct fiemap)
> +#define XFS_IOC_FIEMAPFS			_IOWR('X', 33, struct fiemap)
>  #endif
>  
>  typedef struct histent
> @@ -201,9 +201,9 @@
>  		fiemap->fm_length = length;
>  		fiemap->fm_extent_count = NR_EXTENTS;
>  
> -		ret = ioctl(file->fd, XFS_IOC_FIEMAPFS, (unsigned long)fiemap);
> +		ret = xfsctl(file->name,file->fd, XFS_IOC_FIEMAPFS, (unsigned long)fiemap);
>  		if (ret < 0) {	
> -			fprintf(stderr, "%s: ioctl(FS_IOC_FIEMAPFS) [\"%s\"]: "	
> +			fprintf(stderr, "%s: xfsctl(XFS_IOC_FIEMAPFS) [\"%s\"]: "	
>  				"%s\n", progname, file->name, strerror(errno));

Also, whitespace damage. There's already some in that patch hunk, so
you may as well fix it while you are changing that code.

>  			free(fiemap);
>  			exitcode = 1;
> @@ -338,6 +338,10 @@
>  
>  	if (!init(argc, argv))
>  		return 0;
> +
> +	if (dumpflag)
> +		printf("%8s %8s %8s\n", "agno", "agbno", "len");	

Added functionality? That should be in a separate patch ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH 1/2] xfs_spaceman: XFS_IOC_FIEMAPFS ioctl replacing previous FS_IOC_FIEMAPFS

[Dhruvesh Rathore]

>> --- a/linux-xfs/fs/xfs/xfs_ioctl.c    2015-01-04 15:45:26.518359725 +0530
>> +++ b/linux-xfs/fs/xfs/xfs_ioctl.c    2015-01-04 15:24:06.030407817 +0530
>> @@ -49,6 +49,8 @@
>>  #include <linux/exportfs.h>
>>
>>
>> +/* So that the fiemap access checks can't overflow on 32 bit machines. */
>> +#define FIEMAP_MAX_EXTENTS   (UINT_MAX / sizeof(struct fiemap_extent))
>
> Just move to include/linux/fs.h where the FS_IOC_FIEMAP definitions
> are.

>> +static int fiemap_check_ranges(struct super_block *sb,
>> +                            u64 start, u64 len, u64 *new_len)
>> +{
>> +     u64 maxbytes = (u64) sb->s_maxbytes;
>> +
>> +     *new_len = len;
>> +
>> +     if (len == 0)
>> +             return -EINVAL;
>> +
>> +     if (start > maxbytes)
>> +             return -EFBIG;
>> +
>> +     /*
>> +      * Shrink request scope to what the fs can actually handle.
>> +      */
>> +     if (len > maxbytes || (maxbytes - len) < start)
>> +             *new_len = maxbytes - start;
>>
>> +     return 0;
>> +}
>
> Rather than copying this code, I'd suggest that you remove the "static"
> declaration, export the symbol and add a prototype to
> include/linux/fs.h similar to fiemap_check_flags().
>

>> +static int xfsctl_fiemapfs(struct super_block *sb, unsigned long arg)
>
> - xfs_ioctl_fiemapfs() follows the naming convention used in the
>   rest of the file
> - this is an XFS specific function now, so should pass the xfs_mount
>   rather than the VFS super_block.
> - need a comment saying it was mostly copied from fs/ioctl.c::ioctl_fiemap().
> - arg has already marked as __user converted in the calling
>   function, so we need to retain the __user annotations here.
> - format for XFS functions is this:
>
> /*
>  * Mostly copied from ioctl_fiemap()
>  */
> static int
> xfs_ioctl_fiemapfs(
>         struct xfs_mount        *mp,
>         void __user             *arg)
>
>

>> +     if (!sb->s_op->fiemapfs)
>> +             return -EOPNOTSUPP;
>
> No need to check this - we can call the XFS function directly.
>

>>                       free(fiemap);
>>                       exitcode = 1;
>> @@ -338,6 +338,10 @@
>>
>>       if (!init(argc, argv))
>>               return 0;
>> +
>> +     if (dumpflag)
>> +             printf("%8s %8s %8s\n", "agno", "agbno", "len");
>
> Added functionality? That should be in a separate patch ;)
>

We have assimilated the changes and divided them into three
patches one for the kernel space code, one for the user space
code  and one patch for the above added functionality.

The updated patches have been sent as a separate thread :)

Regards,
A-DRS

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs