[PATCH v2] xfsdump: remove BMV_IF_NO_DMAPI_READ flag

[PATCH v2] xfsdump: remove BMV_IF_NO_DMAPI_READ flag

[Anthony Iliopoulos]

Use of the flag has had no effect since kernel commit 288699fecaff
("xfs: drop dmapi hooks") which removed all dmapi related code, so we
can remove it.

Given that there are no other flags that need to be specified for the
bmap call, convert once instance of it from getbmapx to plain getbmap.

Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
---
changes since v1:
 - convert getbmapx to getbmap

 dump/content.c | 1 -
 dump/inomap.c  | 7 +++----
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/dump/content.c b/dump/content.c
index 75b79220daf6..a40b47101a12 100644
--- a/dump/content.c
+++ b/dump/content.c
@@ -4287,7 +4287,6 @@ init_extent_group_context(jdm_fshandle_t *fshandlep,
 	gcp->eg_bmap[0].bmv_offset = 0;
 	gcp->eg_bmap[0].bmv_length = -1;
 	gcp->eg_bmap[0].bmv_count = BMAP_LEN;
-	gcp->eg_bmap[0].bmv_iflags = BMV_IF_NO_DMAPI_READ;
 	gcp->eg_nextbmapp = &gcp->eg_bmap[1];
 	gcp->eg_endbmapp = &gcp->eg_bmap[1];
 	gcp->eg_bmapix = 0;
diff --git a/dump/inomap.c b/dump/inomap.c
index 85d61c353cf0..f3200be471e0 100644
--- a/dump/inomap.c
+++ b/dump/inomap.c
@@ -1627,7 +1627,7 @@ static off64_t
 quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
 {
 	int fd;
-	getbmapx_t bmap[BMAP_LEN];
+	struct getbmap bmap[BMAP_LEN];
 	off64_t offset;
 	off64_t offset_next;
 	off64_t qty_accum;
@@ -1647,7 +1647,6 @@ quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
 	bmap[0].bmv_offset = 0;
 	bmap[0].bmv_length = -1;
 	bmap[0].bmv_count = BMAP_LEN;
-	bmap[0].bmv_iflags = BMV_IF_NO_DMAPI_READ;
 	bmap[0].bmv_entries = -1;
 	fd = jdm_open(fshandlep, statp, O_RDONLY);
 	if (fd < 0) {
@@ -1662,7 +1661,7 @@ quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
 		int eix;
 		int rval;
 
-		rval = ioctl(fd, XFS_IOC_GETBMAPX, bmap);
+		rval = ioctl(fd, XFS_IOC_GETBMAP, bmap);
 		if (rval) {
 			mlog(MLOG_NORMAL | MLOG_WARNING | MLOG_INOMAP, _(
 			      "could not read extent map for ino %llu: %s\n"),
@@ -1679,7 +1678,7 @@ quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
 		}
 
 		for (eix = 1; eix <= bmap[0].bmv_entries; eix++) {
-			getbmapx_t *bmapp = &bmap[eix];
+			struct getbmap *bmapp = &bmap[eix];
 			off64_t qty_new;
 			if (bmapp->bmv_block == -1) {
 				continue; /* hole */
-- 
2.31.0

Re: [PATCH v2] xfsdump: remove BMV_IF_NO_DMAPI_READ flag

[Christoph Hellwig]

On Wed, Mar 31, 2021 at 06:26:17PM +0200, Anthony Iliopoulos wrote:
> Use of the flag has had no effect since kernel commit 288699fecaff
> ("xfs: drop dmapi hooks") which removed all dmapi related code, so we
> can remove it.
> 
> Given that there are no other flags that need to be specified for the
> bmap call, convert once instance of it from getbmapx to plain getbmap.
> 
> Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>

I'm not sure dorpping this as long as xfsdump still has all kinds of
other DMAPI related code..

Re: [PATCH v2] xfsdump: remove BMV_IF_NO_DMAPI_READ flag

[Carlos Maiolino]

Reviving this old patch since changes have been pushed to xfsprogs.

On Wed, Mar 31, 2021 at 06:26:17PM +0200, Anthony Iliopoulos wrote:
> Use of the flag has had no effect since kernel commit 288699fecaff
> ("xfs: drop dmapi hooks") which removed all dmapi related code, so we
> can remove it.
> 
> Given that there are no other flags that need to be specified for the
> bmap call, convert once instance of it from getbmapx to plain getbmap.
> 
> Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
> ---
> changes since v1:
>  - convert getbmapx to getbmap
> 
>  dump/content.c | 1 -
>  dump/inomap.c  | 7 +++----
>  2 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/dump/content.c b/dump/content.c
> index 75b79220daf6..a40b47101a12 100644
> --- a/dump/content.c
> +++ b/dump/content.c
> @@ -4287,7 +4287,6 @@ init_extent_group_context(jdm_fshandle_t *fshandlep,
>  	gcp->eg_bmap[0].bmv_offset = 0;
>  	gcp->eg_bmap[0].bmv_length = -1;
>  	gcp->eg_bmap[0].bmv_count = BMAP_LEN;
> -	gcp->eg_bmap[0].bmv_iflags = BMV_IF_NO_DMAPI_READ;
>  	gcp->eg_nextbmapp = &gcp->eg_bmap[1];
>  	gcp->eg_endbmapp = &gcp->eg_bmap[1];
>  	gcp->eg_bmapix = 0;
> diff --git a/dump/inomap.c b/dump/inomap.c
> index 85d61c353cf0..f3200be471e0 100644
> --- a/dump/inomap.c
> +++ b/dump/inomap.c
> @@ -1627,7 +1627,7 @@ static off64_t
>  quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
>  {
>  	int fd;
> -	getbmapx_t bmap[BMAP_LEN];
> +	struct getbmap bmap[BMAP_LEN];

This seems ok, but I still think this should be properly initialized here, like:

struct getbmap bmap[BMAP_LEN] = {0};


>  	off64_t offset;
>  	off64_t offset_next;
>  	off64_t qty_accum;
> @@ -1647,7 +1647,6 @@ quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
>  	bmap[0].bmv_offset = 0;
>  	bmap[0].bmv_length = -1;
>  	bmap[0].bmv_count = BMAP_LEN;
> -	bmap[0].bmv_iflags = BMV_IF_NO_DMAPI_READ;
>  	bmap[0].bmv_entries = -1;
>  	fd = jdm_open(fshandlep, statp, O_RDONLY);
>  	if (fd < 0) {
> @@ -1662,7 +1661,7 @@ quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
>  		int eix;
>  		int rval;
>  
> -		rval = ioctl(fd, XFS_IOC_GETBMAPX, bmap);
> +		rval = ioctl(fd, XFS_IOC_GETBMAP, bmap);
>  		if (rval) {
>  			mlog(MLOG_NORMAL | MLOG_WARNING | MLOG_INOMAP, _(
>  			      "could not read extent map for ino %llu: %s\n"),
> @@ -1679,7 +1678,7 @@ quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
>  		}
>  
>  		for (eix = 1; eix <= bmap[0].bmv_entries; eix++) {
> -			getbmapx_t *bmapp = &bmap[eix];
> +			struct getbmap *bmapp = &bmap[eix];
>  			off64_t qty_new;
>  			if (bmapp->bmv_block == -1) {
>  				continue; /* hole */
> -- 
> 2.31.0
> 

-- 
Carlos

Re: [PATCH v2] xfsdump: remove BMV_IF_NO_DMAPI_READ flag

[Carlos Maiolino]

On Wed, Mar 31, 2021 at 06:26:17PM +0200, Anthony Iliopoulos wrote:
> Use of the flag has had no effect since kernel commit 288699fecaff
> ("xfs: drop dmapi hooks") which removed all dmapi related code, so we
> can remove it.
> 
> Given that there are no other flags that need to be specified for the
> bmap call, convert once instance of it from getbmapx to plain getbmap.
> 
> Signed-off-by: Anthony Iliopoulos <ailiop@suse.com>
> ---
> changes since v1:
>  - convert getbmapx to getbmap
> 
>  dump/content.c | 1 -
>  dump/inomap.c  | 7 +++----
>  2 files changed, 3 insertions(+), 5 deletions(-)

With my follow-up patch applied on top of this:

Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>

> 
> diff --git a/dump/content.c b/dump/content.c
> index 75b79220daf6..a40b47101a12 100644
> --- a/dump/content.c
> +++ b/dump/content.c
> @@ -4287,7 +4287,6 @@ init_extent_group_context(jdm_fshandle_t *fshandlep,
>  	gcp->eg_bmap[0].bmv_offset = 0;
>  	gcp->eg_bmap[0].bmv_length = -1;
>  	gcp->eg_bmap[0].bmv_count = BMAP_LEN;
> -	gcp->eg_bmap[0].bmv_iflags = BMV_IF_NO_DMAPI_READ;
>  	gcp->eg_nextbmapp = &gcp->eg_bmap[1];
>  	gcp->eg_endbmapp = &gcp->eg_bmap[1];
>  	gcp->eg_bmapix = 0;
> diff --git a/dump/inomap.c b/dump/inomap.c
> index 85d61c353cf0..f3200be471e0 100644
> --- a/dump/inomap.c
> +++ b/dump/inomap.c
> @@ -1627,7 +1627,7 @@ static off64_t
>  quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
>  {
>  	int fd;
> -	getbmapx_t bmap[BMAP_LEN];
> +	struct getbmap bmap[BMAP_LEN];
>  	off64_t offset;
>  	off64_t offset_next;
>  	off64_t qty_accum;
> @@ -1647,7 +1647,6 @@ quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
>  	bmap[0].bmv_offset = 0;
>  	bmap[0].bmv_length = -1;
>  	bmap[0].bmv_count = BMAP_LEN;
> -	bmap[0].bmv_iflags = BMV_IF_NO_DMAPI_READ;
>  	bmap[0].bmv_entries = -1;
>  	fd = jdm_open(fshandlep, statp, O_RDONLY);
>  	if (fd < 0) {
> @@ -1662,7 +1661,7 @@ quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
>  		int eix;
>  		int rval;
>  
> -		rval = ioctl(fd, XFS_IOC_GETBMAPX, bmap);
> +		rval = ioctl(fd, XFS_IOC_GETBMAP, bmap);
>  		if (rval) {
>  			mlog(MLOG_NORMAL | MLOG_WARNING | MLOG_INOMAP, _(
>  			      "could not read extent map for ino %llu: %s\n"),
> @@ -1679,7 +1678,7 @@ quantity2offset(jdm_fshandle_t *fshandlep, struct xfs_bstat *statp, off64_t qty)
>  		}
>  
>  		for (eix = 1; eix <= bmap[0].bmv_entries; eix++) {
> -			getbmapx_t *bmapp = &bmap[eix];
> +			struct getbmap *bmapp = &bmap[eix];
>  			off64_t qty_new;
>  			if (bmapp->bmv_block == -1) {
>  				continue; /* hole */
> -- 
> 2.31.0
> 

-- 
Carlos