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

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

[Anand Jain]

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(-)

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

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

[Marc MERLIN]

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
> 

-- 
"A mouse is a device used to point at the xterm you want to type in" - A.S.R.
Microsoft is to operating systems ....
                                      .... what McDonalds is to gourmet cooking
Home page: http://marc.merlins.org/  

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

[David Sterba]

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.

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

[Anand Jain]

On 04/14/2015 09:14 PM, David Sterba wrote:
> 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.

You means res not ret (above) ?
--------------------------------------------------------------
diff --git a/cmds-device.c b/cmds-device.c
index cbb3243..1022656 100644
--- a/cmds-device.c
+++ b/cmds-device.c
@@ -183,7 +183,7 @@ static int cmd_rm_dev(int argc, char **argv)
                 if (res) {
                         const char *msg;

-                       if (ret > 0)
+                       if (res > 0)
                                 msg = btrfs_err_str(res);
                         else
                                 msg = strerror(e);
--------------------------------------------------------------

Thanks, Anand


> --
> 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
>

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

[David Sterba]

On Fri, May 08, 2015 at 04:28:24PM +0800, Anand Jain wrote:
> >> @@ -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.
> 
> You means res not ret (above) ?

Oh right, please send a patch.