[1/3] btrfs-progs: check: report more specific info about invalid location
diff mbox

Message ID 20171121101524.2014-1-suy.fnst@cn.fujitsu.com
State New
Headers show

Commit Message

Su Yue Nov. 21, 2017, 10:15 a.m. UTC
Previously, it was so useless to print message like
"invalid location %d".

Let it print objectid and offset of dir_item.
Debug is easier now.

Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
---
 cmds-check.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Nikolay Borisov Nov. 21, 2017, 11:46 a.m. UTC | #1
On 21.11.2017 12:15, Su Yue wrote:
> Previously, it was so useless to print message like
> "invalid location %d".
> 
> Let it print objectid and offset of dir_item.
> Debug is easier now.
> 
> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
> ---
>  cmds-check.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/cmds-check.c b/cmds-check.c
> index a93ac2c88a38..5c15dfb60b9a 100644
> --- a/cmds-check.c
> +++ b/cmds-check.c
> @@ -1635,8 +1635,8 @@ static int process_dir_item(struct extent_buffer *eb,
>  					  namebuf, len, filetype,
>  					  key->type, error);
>  		} else {
> -			fprintf(stderr, "invalid location in dir item %u\n",
> -				location.type);
> +			fprintf(stderr, "invalid location in dir item[%llu %llu]\n",
> +				key->objectid, key->offset);

I think it will be good if in addition to the dir item's key you also
print the invalid type value. I.e. this check is triggered if
location.type is unrecognizable. So the error message could be something
like :

fprintf(stderr, "Unrecognised location.type (%u) in DIR_ITEM[%llu
%llu]", key->objectid, key->offset);

Let's not force the user to go and read the code to understand why this
error happened.


>  			add_inode_backref(inode_cache, BTRFS_MULTIPLE_OBJECTIDS,
>  					  key->objectid, key->offset, namebuf,
>  					  len, filetype, key->type, error);
> 
--
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
Su Yue Nov. 22, 2017, 1:48 a.m. UTC | #2
On 11/21/2017 07:46 PM, Nikolay Borisov wrote:
> 
> 
> On 21.11.2017 12:15, Su Yue wrote:
>> Previously, it was so useless to print message like
>> "invalid location %d".
>>
>> Let it print objectid and offset of dir_item.
>> Debug is easier now.
>>
>> Signed-off-by: Su Yue <suy.fnst@cn.fujitsu.com>
>> ---
>>   cmds-check.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/cmds-check.c b/cmds-check.c
>> index a93ac2c88a38..5c15dfb60b9a 100644
>> --- a/cmds-check.c
>> +++ b/cmds-check.c
>> @@ -1635,8 +1635,8 @@ static int process_dir_item(struct extent_buffer *eb,
>>   					  namebuf, len, filetype,
>>   					  key->type, error);
>>   		} else {
>> -			fprintf(stderr, "invalid location in dir item %u\n",
>> -				location.type);
>> +			fprintf(stderr, "invalid location in dir item[%llu %llu]\n",
>> +				key->objectid, key->offset);
> 
> I think it will be good if in addition to the dir item's key you also
> print the invalid type value. I.e. this check is triggered if
> location.type is unrecognizable. So the error message could be something
> like :
> 
> fprintf(stderr, "Unrecognised location.type (%u) in DIR_ITEM[%llu
> %llu]", key->objectid, key->offset);
> 
Thanks. Printing location type is more specific.
I will update the patch.

Thanks,
Su
> Let's not force the user to go and read the code to understand why this
> error happened.
> 
> 
>>   			add_inode_backref(inode_cache, BTRFS_MULTIPLE_OBJECTIDS,
>>   					  key->objectid, key->offset, namebuf,
>>   					  len, filetype, key->type, error);
>>
> 
> 


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

Patch
diff mbox

diff --git a/cmds-check.c b/cmds-check.c
index a93ac2c88a38..5c15dfb60b9a 100644
--- a/cmds-check.c
+++ b/cmds-check.c
@@ -1635,8 +1635,8 @@  static int process_dir_item(struct extent_buffer *eb,
 					  namebuf, len, filetype,
 					  key->type, error);
 		} else {
-			fprintf(stderr, "invalid location in dir item %u\n",
-				location.type);
+			fprintf(stderr, "invalid location in dir item[%llu %llu]\n",
+				key->objectid, key->offset);
 			add_inode_backref(inode_cache, BTRFS_MULTIPLE_OBJECTIDS,
 					  key->objectid, key->offset, namebuf,
 					  len, filetype, key->type, error);