[PATCHSET 0/2] xfs_io: small fixes to funshare command

[PATCHSET 0/2] xfs_io: small fixes to funshare command

[Darrick J. Wong]

Hi all,

A couple of small fixes to the funshare command.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=funshare-fixes
---
 io/prealloc.c |   33 +++++++++++++++++++++------------
 1 file changed, 21 insertions(+), 12 deletions(-)

[PATCH 1/2] xfs_io: fix broken funshare_cmd usage

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Create a funshare_cmd and use that to store information about the
xfs_io funshare command instead of overwriting the contents of
fzero_cmd.  This fixes confusing output like:

$ xfs_io -c 'fzero 2 3 --help' /
fzero: invalid option -- '-'
funshare off len -- unshares shared blocks within the range

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 io/prealloc.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)


diff --git a/io/prealloc.c b/io/prealloc.c
index 382e8119..2ae8afe9 100644
--- a/io/prealloc.c
+++ b/io/prealloc.c
@@ -43,6 +43,7 @@ static cmdinfo_t fpunch_cmd;
 static cmdinfo_t fcollapse_cmd;
 static cmdinfo_t finsert_cmd;
 static cmdinfo_t fzero_cmd;
+static cmdinfo_t funshare_cmd;
 #endif
 
 static int
@@ -467,14 +468,14 @@ prealloc_init(void)
 	_("zeroes space and eliminates holes by preallocating");
 	add_command(&fzero_cmd);
 
-	fzero_cmd.name = "funshare";
-	fzero_cmd.cfunc = funshare_f;
-	fzero_cmd.argmin = 2;
-	fzero_cmd.argmax = 2;
-	fzero_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	fzero_cmd.args = _("off len");
-	fzero_cmd.oneline =
+	funshare_cmd.name = "funshare";
+	funshare_cmd.cfunc = funshare_f;
+	funshare_cmd.argmin = 2;
+	funshare_cmd.argmax = 2;
+	funshare_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	funshare_cmd.args = _("off len");
+	funshare_cmd.oneline =
 	_("unshares shared blocks within the range");
-	add_command(&fzero_cmd);
+	add_command(&funshare_cmd);
 #endif	/* HAVE_FALLOCATE */
 }

[PATCH 2/2] xfs_io: clean up the funshare command a bit

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Add proper argument parsing to the funshare command so that when you
pass it nonexistent --help it will print the help instead of complaining
that it can't convert that to a number.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
---
 io/prealloc.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)


diff --git a/io/prealloc.c b/io/prealloc.c
index 2ae8afe9..a8831c1b 100644
--- a/io/prealloc.c
+++ b/io/prealloc.c
@@ -346,16 +346,24 @@ funshare_f(
 	char		**argv)
 {
 	xfs_flock64_t	segment;
+	int		c;
 	int		mode = FALLOC_FL_UNSHARE_RANGE;
-	int		index = 1;
 
-	if (!offset_length(argv[index], argv[index + 1], &segment)) {
+	while ((c = getopt(argc, argv, "")) != EOF) {
+		switch (c) {
+		default:
+			command_usage(&funshare_cmd);
+		}
+	}
+	if (optind != argc - 2)
+		return command_usage(&funshare_cmd);
+
+	if (!offset_length(argv[optind], argv[optind + 1], &segment)) {
 		exitcode = 1;
 		return 0;
 	}
 
-	if (fallocate(file->fd, mode,
-			segment.l_start, segment.l_len)) {
+	if (fallocate(file->fd, mode, segment.l_start, segment.l_len)) {
 		perror("fallocate");
 		exitcode = 1;
 		return 0;

Re: [PATCH 1/2] xfs_io: fix broken funshare_cmd usage

[Carlos Maiolino]

On Fri, Jul 02, 2021 at 07:58:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a funshare_cmd and use that to store information about the
> xfs_io funshare command instead of overwriting the contents of
> fzero_cmd.  This fixes confusing output like:
> 
> $ xfs_io -c 'fzero 2 3 --help' /
> fzero: invalid option -- '-'
> funshare off len -- unshares shared blocks within the range

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


> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  io/prealloc.c |   17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/io/prealloc.c b/io/prealloc.c
> index 382e8119..2ae8afe9 100644
> --- a/io/prealloc.c
> +++ b/io/prealloc.c
> @@ -43,6 +43,7 @@ static cmdinfo_t fpunch_cmd;
>  static cmdinfo_t fcollapse_cmd;
>  static cmdinfo_t finsert_cmd;
>  static cmdinfo_t fzero_cmd;
> +static cmdinfo_t funshare_cmd;
>  #endif
>  
>  static int
> @@ -467,14 +468,14 @@ prealloc_init(void)
>  	_("zeroes space and eliminates holes by preallocating");
>  	add_command(&fzero_cmd);
>  
> -	fzero_cmd.name = "funshare";
> -	fzero_cmd.cfunc = funshare_f;
> -	fzero_cmd.argmin = 2;
> -	fzero_cmd.argmax = 2;
> -	fzero_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> -	fzero_cmd.args = _("off len");
> -	fzero_cmd.oneline =
> +	funshare_cmd.name = "funshare";
> +	funshare_cmd.cfunc = funshare_f;
> +	funshare_cmd.argmin = 2;
> +	funshare_cmd.argmax = 2;
> +	funshare_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
> +	funshare_cmd.args = _("off len");
> +	funshare_cmd.oneline =
>  	_("unshares shared blocks within the range");
> -	add_command(&fzero_cmd);
> +	add_command(&funshare_cmd);
>  #endif	/* HAVE_FALLOCATE */
>  }
> 

-- 
Carlos

Re: [PATCH 1/2] xfs_io: fix broken funshare_cmd usage

[Darrick J. Wong]

On Mon, Jul 05, 2021 at 04:16:42PM +0100, Christoph Hellwig wrote:
> On Fri, Jul 02, 2021 at 07:58:02PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Create a funshare_cmd and use that to store information about the
> > xfs_io funshare command instead of overwriting the contents of
> > fzero_cmd.  This fixes confusing output like:
> > 
> > $ xfs_io -c 'fzero 2 3 --help' /
> > fzero: invalid option -- '-'
> > funshare off len -- unshares shared blocks within the range
> 
> Ooops, how did this manage to ever work?

It "works" (in the sense that fzero and funshare issue the correct
fallocate modes) because add_command copies the contents of its struct
parameter into the internal command list.  The braindamage is limited to
any subsequent use of fzero_cmd, which (afaict) means the only way you'd
notice is through the help screens.

--D

> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 1/2] xfs_io: fix broken funshare_cmd usage

[Christoph Hellwig]

On Fri, Jul 02, 2021 at 07:58:02PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a funshare_cmd and use that to store information about the
> xfs_io funshare command instead of overwriting the contents of
> fzero_cmd.  This fixes confusing output like:
> 
> $ xfs_io -c 'fzero 2 3 --help' /
> fzero: invalid option -- '-'
> funshare off len -- unshares shared blocks within the range

Ooops, how did this manage to ever work?

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 1/2] xfs_io: fix broken funshare_cmd usage

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Create a funshare_cmd and use that to store information about the
xfs_io funshare command instead of overwriting the contents of
fzero_cmd.  This fixes confusing output like:

$ xfs_io -c 'fzero 2 3 --help' /
fzero: invalid option -- '-'
funshare off len -- unshares shared blocks within the range

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 io/prealloc.c |   17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)


diff --git a/io/prealloc.c b/io/prealloc.c
index 382e8119..2ae8afe9 100644
--- a/io/prealloc.c
+++ b/io/prealloc.c
@@ -43,6 +43,7 @@ static cmdinfo_t fpunch_cmd;
 static cmdinfo_t fcollapse_cmd;
 static cmdinfo_t finsert_cmd;
 static cmdinfo_t fzero_cmd;
+static cmdinfo_t funshare_cmd;
 #endif
 
 static int
@@ -467,14 +468,14 @@ prealloc_init(void)
 	_("zeroes space and eliminates holes by preallocating");
 	add_command(&fzero_cmd);
 
-	fzero_cmd.name = "funshare";
-	fzero_cmd.cfunc = funshare_f;
-	fzero_cmd.argmin = 2;
-	fzero_cmd.argmax = 2;
-	fzero_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
-	fzero_cmd.args = _("off len");
-	fzero_cmd.oneline =
+	funshare_cmd.name = "funshare";
+	funshare_cmd.cfunc = funshare_f;
+	funshare_cmd.argmin = 2;
+	funshare_cmd.argmax = 2;
+	funshare_cmd.flags = CMD_NOMAP_OK | CMD_FOREIGN_OK;
+	funshare_cmd.args = _("off len");
+	funshare_cmd.oneline =
 	_("unshares shared blocks within the range");
-	add_command(&fzero_cmd);
+	add_command(&funshare_cmd);
 #endif	/* HAVE_FALLOCATE */
 }