diff mbox

[1/1] btrfs-progs: improve troubleshooting avoid duplicate error strings

Message ID 1428928621-8001-1-git-send-email-anand.jain@oracle.com (mailing list archive)
State Accepted
Headers show

Commit Message

Anand Jain April 13, 2015, 12:37 p.m. UTC
my troubleshooting experience says have unique error string per module.

In the below eg, its one additional step to know error line,

cat -n cmds-device.c | egrep "error removing the device"
   185	"ERROR: error removing the device '%s' - %s\n",
   190	"ERROR: error removing the device '%s' - %s\n",

which is completely avoidable.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 cmds-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Marc MERLIN April 13, 2015, 1:40 p.m. UTC | #1
On Mon, Apr 13, 2015 at 08:37:01PM +0800, Anand Jain wrote:
> my troubleshooting experience says have unique error string per module.

+1 to that, thank you.

Marc

> In the below eg, its one additional step to know error line,
> 
> cat -n cmds-device.c | egrep "error removing the device"
>    185	"ERROR: error removing the device '%s' - %s\n",
>    190	"ERROR: error removing the device '%s' - %s\n",
> 
> which is completely avoidable.
> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  cmds-device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmds-device.c b/cmds-device.c
> index 1c72e90..1c32771 100644
> --- a/cmds-device.c
> +++ b/cmds-device.c
> @@ -187,7 +187,7 @@ static int cmd_rm_dev(int argc, char **argv)
>  			ret++;
>  		} else if (res < 0) {
>  			fprintf(stderr,
> -				"ERROR: error removing the device '%s' - %s\n",
> +				"ERROR: ioctl error removing the device '%s' - %s\n",
>  				argv[i], strerror(e));
>  			ret++;
>  		}
> -- 
> 2.0.0.153.g79dcccc
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
David Sterba April 14, 2015, 1:14 p.m. UTC | #2
On Mon, Apr 13, 2015 at 08:37:01PM +0800, Anand Jain wrote:
> my troubleshooting experience says have unique error string per module.
> 
> In the below eg, its one additional step to know error line,
> 
> cat -n cmds-device.c | egrep "error removing the device"
>    185	"ERROR: error removing the device '%s' - %s\n",
>    190	"ERROR: error removing the device '%s' - %s\n",
> 
> which is completely avoidable.

It is, we can merge both branches into one.

> 
> Signed-off-by: Anand Jain <anand.jain@oracle.com>
> ---
>  cmds-device.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/cmds-device.c b/cmds-device.c
> index 1c72e90..1c32771 100644
> --- a/cmds-device.c
> +++ b/cmds-device.c
> @@ -187,7 +187,7 @@ static int cmd_rm_dev(int argc, char **argv)
>  			ret++;
>  		} else if (res < 0) {
>  			fprintf(stderr,
> -				"ERROR: error removing the device '%s' - %s\n",
> +				"ERROR: ioctl error removing the device '%s' - %s\n",

The only difference is the strerror vs btrfs_err_str. As both ret > 0
and ret < 0 report some kind of error, the wording would be very similar
so I think that one error message would fit better. I'll fix that.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/cmds-device.c b/cmds-device.c
index 1c72e90..1c32771 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -187,7 +187,7 @@  static int cmd_rm_dev(int argc, char **argv)
 			ret++;
 		} else if (res < 0) {
 			fprintf(stderr,
-				"ERROR: error removing the device '%s' - %s\n",
+				"ERROR: ioctl error removing the device '%s' - %s\n",
 				argv[i], strerror(e));
 			ret++;
 		}