| Message ID | 20171031040341.18840-1-wqu@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2017/10/31 13:03, Qu Wenruo wrote: > For key type BTRFS_UUID_KEY_SUBVOL or BTRFS_UUID_KEY_RECEIVED_SUBVOL the > key objectid and key offset are just half of the UUID. > > However we just print the key as %llu, which is converted from little > endian, not byte order for UUID, nor the traditional 36 bytes human > readable uuid format. > > Although true engineer can easily convert it in their brain, but to > make it easier for search, output the result UUID using the 36 chars format. > > Cc: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > Inspired by UUID related work from Misono. > --- > print-tree.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/print-tree.c b/print-tree.c > index 3c585e31f1fc..687f871db302 100644 > --- a/print-tree.c > +++ b/print-tree.c > @@ -803,14 +803,25 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key) > } > } > > -static void print_uuid_item(struct extent_buffer *l, unsigned long offset, > - u32 item_size) > +static void print_uuid_item(struct extent_buffer *l, int slot, > + unsigned long offset, u32 item_size) > { > + struct btrfs_key key; > + char uuid_str[BTRFS_UUID_UNPARSED_SIZE]; > + u8 uuid[BTRFS_UUID_SIZE]; > + > + /* Reassemble the uuid from key.objecitd and key.offset */ > + btrfs_item_key_to_cpu(l, &key, slot); > + put_unaligned_le64(key.objectid, uuid); > + put_unaligned_le64(key.offset, uuid + sizeof(u64)); > + uuid_unparse(uuid, uuid_str); > + > if (item_size & (sizeof(u64) - 1)) { > printf("btrfs: uuid item with illegal size %lu!\n", > (unsigned long)item_size); > return; > } > + printf("\t\tuuid %s\n", uuid_str); > while (item_size) { > __le64 subvol_id; > > @@ -1297,7 +1308,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb) > break; > case BTRFS_UUID_KEY_SUBVOL: > case BTRFS_UUID_KEY_RECEIVED_SUBVOL: > - print_uuid_item(eb, btrfs_item_ptr_offset(eb, i), > + print_uuid_item(eb, i, btrfs_item_ptr_offset(eb, i), > btrfs_item_size_nr(eb, i)); > break; > case BTRFS_STRING_ITEM_KEY: { > Reviewed-by: Tomohiro Misono <misono.tomohiro@jp.fujitsu.com> -- 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
On 31.10.2017 06:03, Qu Wenruo wrote: > For key type BTRFS_UUID_KEY_SUBVOL or BTRFS_UUID_KEY_RECEIVED_SUBVOL the > key objectid and key offset are just half of the UUID. > > However we just print the key as %llu, which is converted from little > endian, not byte order for UUID, nor the traditional 36 bytes human > readable uuid format. > > Although true engineer can easily convert it in their brain, but to > make it easier for search, output the result UUID using the 36 chars format. > > Cc: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > Inspired by UUID related work from Misono. > --- > print-tree.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/print-tree.c b/print-tree.c > index 3c585e31f1fc..687f871db302 100644 > --- a/print-tree.c > +++ b/print-tree.c > @@ -803,14 +803,25 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key) > } > } > > -static void print_uuid_item(struct extent_buffer *l, unsigned long offset, > - u32 item_size) > +static void print_uuid_item(struct extent_buffer *l, int slot, > + unsigned long offset, u32 item_size) > { > + struct btrfs_key key; > + char uuid_str[BTRFS_UUID_UNPARSED_SIZE]; > + u8 uuid[BTRFS_UUID_SIZE]; > + > + /* Reassemble the uuid from key.objecitd and key.offset */ > + btrfs_item_key_to_cpu(l, &key, slot); > + put_unaligned_le64(key.objectid, uuid); > + put_unaligned_le64(key.offset, uuid + sizeof(u64)); I don't think this will work on a BE system. Because btrfs_item_key_to_cpu take the LE representation on-disk and turns it into a cpu representation which might very well be BE. And then you essentially reverse it by using put_unaligned_le64 for x86 it works fine due to it being a LE system. > + uuid_unparse(uuid, uuid_str); > + > if (item_size & (sizeof(u64) - 1)) { > printf("btrfs: uuid item with illegal size %lu!\n", > (unsigned long)item_size); > return; > } > + printf("\t\tuuid %s\n", uuid_str); > while (item_size) { > __le64 subvol_id; > > @@ -1297,7 +1308,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb) > break; > case BTRFS_UUID_KEY_SUBVOL: > case BTRFS_UUID_KEY_RECEIVED_SUBVOL: > - print_uuid_item(eb, btrfs_item_ptr_offset(eb, i), > + print_uuid_item(eb, i, btrfs_item_ptr_offset(eb, i), > btrfs_item_size_nr(eb, i)); > break; > case BTRFS_STRING_ITEM_KEY: { > -- 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
On 2017年10月31日 15:15, Nikolay Borisov wrote: > > > On 31.10.2017 06:03, Qu Wenruo wrote: >> For key type BTRFS_UUID_KEY_SUBVOL or BTRFS_UUID_KEY_RECEIVED_SUBVOL the >> key objectid and key offset are just half of the UUID. >> >> However we just print the key as %llu, which is converted from little >> endian, not byte order for UUID, nor the traditional 36 bytes human >> readable uuid format. >> >> Although true engineer can easily convert it in their brain, but to >> make it easier for search, output the result UUID using the 36 chars format. >> >> Cc: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> Inspired by UUID related work from Misono. >> --- >> print-tree.c | 17 ++++++++++++++--- >> 1 file changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/print-tree.c b/print-tree.c >> index 3c585e31f1fc..687f871db302 100644 >> --- a/print-tree.c >> +++ b/print-tree.c >> @@ -803,14 +803,25 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key) >> } >> } >> >> -static void print_uuid_item(struct extent_buffer *l, unsigned long offset, >> - u32 item_size) >> +static void print_uuid_item(struct extent_buffer *l, int slot, >> + unsigned long offset, u32 item_size) >> { >> + struct btrfs_key key; >> + char uuid_str[BTRFS_UUID_UNPARSED_SIZE]; >> + u8 uuid[BTRFS_UUID_SIZE]; >> + >> + /* Reassemble the uuid from key.objecitd and key.offset */ >> + btrfs_item_key_to_cpu(l, &key, slot); >> + put_unaligned_le64(key.objectid, uuid); >> + put_unaligned_le64(key.offset, uuid + sizeof(u64)); > > I don't think this will work on a BE system. Because > btrfs_item_key_to_cpu take the LE representation on-disk and turns it > into a cpu representation which might very well be BE. And then you > essentially reverse it by using put_unaligned_le64 for x86 it works fine > due to it being a LE system. I know this can be tricky, let's assume the following case: UUID: 0x0123456789abcdef0123456789abcdef (byte order, no endian) Low bit high bit Key objectid: 0x0123456789abcdef (LE on-disk) 0xefcdab8967452301 (u64) <- CPU key.objectid Low bit hight bit key.offset: 0x123456789abcdef (LE on-disk) 0xefcdab8967452301 (u64) <- CPU key.offset put_unaligned_le64 will convert CPU key.objectid/offset to LE on-disk again, so we get Low bit high bit 0x01 23456789abcd ef (LE on-disk) uuid[0] ... uuid[7] And that's what we need. We did the LE->native and native->LE, so the result is not changed at all. And we just need byte order, so the result is correct. Just like what kernel did. Thanks, Qu > > >> + uuid_unparse(uuid, uuid_str); >> + >> if (item_size & (sizeof(u64) - 1)) { >> printf("btrfs: uuid item with illegal size %lu!\n", >> (unsigned long)item_size); >> return; >> } >> + printf("\t\tuuid %s\n", uuid_str); >> while (item_size) { >> __le64 subvol_id; >> >> @@ -1297,7 +1308,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb) >> break; >> case BTRFS_UUID_KEY_SUBVOL: >> case BTRFS_UUID_KEY_RECEIVED_SUBVOL: >> - print_uuid_item(eb, btrfs_item_ptr_offset(eb, i), >> + print_uuid_item(eb, i, btrfs_item_ptr_offset(eb, i), >> btrfs_item_size_nr(eb, i)); >> break; >> case BTRFS_STRING_ITEM_KEY: { >> > -- > 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 >
On 31.10.2017 09:15, Nikolay Borisov wrote: > > > On 31.10.2017 06:03, Qu Wenruo wrote: >> For key type BTRFS_UUID_KEY_SUBVOL or BTRFS_UUID_KEY_RECEIVED_SUBVOL the >> key objectid and key offset are just half of the UUID. >> >> However we just print the key as %llu, which is converted from little >> endian, not byte order for UUID, nor the traditional 36 bytes human >> readable uuid format. >> >> Although true engineer can easily convert it in their brain, but to >> make it easier for search, output the result UUID using the 36 chars format. >> >> Cc: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> >> Signed-off-by: Qu Wenruo <wqu@suse.com> >> --- >> Inspired by UUID related work from Misono. >> --- >> print-tree.c | 17 ++++++++++++++--- >> 1 file changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/print-tree.c b/print-tree.c >> index 3c585e31f1fc..687f871db302 100644 >> --- a/print-tree.c >> +++ b/print-tree.c >> @@ -803,14 +803,25 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key) >> } >> } >> >> -static void print_uuid_item(struct extent_buffer *l, unsigned long offset, >> - u32 item_size) >> +static void print_uuid_item(struct extent_buffer *l, int slot, >> + unsigned long offset, u32 item_size) >> { >> + struct btrfs_key key; >> + char uuid_str[BTRFS_UUID_UNPARSED_SIZE]; >> + u8 uuid[BTRFS_UUID_SIZE]; >> + >> + /* Reassemble the uuid from key.objecitd and key.offset */ >> + btrfs_item_key_to_cpu(l, &key, slot); >> + put_unaligned_le64(key.objectid, uuid); >> + put_unaligned_le64(key.offset, uuid + sizeof(u64)); > > I don't think this will work on a BE system. Because > btrfs_item_key_to_cpu take the LE representation on-disk and turns it > into a cpu representation which might very well be BE. And then you > essentially reverse it by using put_unaligned_le64 for x86 it works fine > due to it being a LE system. Ok, so looking at one of your other patches and some digging seems to indicate that btrfs explicitly generates LE uuids so your code is correct, however it's not obvoious from this patch itself. I suggest to put either a comment above the put_unaligned or a statement in the commit message that uuids are always generated in little-endian format > > >> + uuid_unparse(uuid, uuid_str); >> + >> if (item_size & (sizeof(u64) - 1)) { >> printf("btrfs: uuid item with illegal size %lu!\n", >> (unsigned long)item_size); >> return; >> } >> + printf("\t\tuuid %s\n", uuid_str); >> while (item_size) { >> __le64 subvol_id; >> >> @@ -1297,7 +1308,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb) >> break; >> case BTRFS_UUID_KEY_SUBVOL: >> case BTRFS_UUID_KEY_RECEIVED_SUBVOL: >> - print_uuid_item(eb, btrfs_item_ptr_offset(eb, i), >> + print_uuid_item(eb, i, btrfs_item_ptr_offset(eb, i), >> btrfs_item_size_nr(eb, i)); >> break; >> case BTRFS_STRING_ITEM_KEY: { >> > -- > 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 > -- 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
On 2017年10月31日 15:29, Nikolay Borisov wrote: > > > On 31.10.2017 09:15, Nikolay Borisov wrote: >> >> >> On 31.10.2017 06:03, Qu Wenruo wrote: >>> For key type BTRFS_UUID_KEY_SUBVOL or BTRFS_UUID_KEY_RECEIVED_SUBVOL the >>> key objectid and key offset are just half of the UUID. >>> >>> However we just print the key as %llu, which is converted from little >>> endian, not byte order for UUID, nor the traditional 36 bytes human >>> readable uuid format. >>> >>> Although true engineer can easily convert it in their brain, but to >>> make it easier for search, output the result UUID using the 36 chars format. >>> >>> Cc: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> >>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>> --- >>> Inspired by UUID related work from Misono. >>> --- >>> print-tree.c | 17 ++++++++++++++--- >>> 1 file changed, 14 insertions(+), 3 deletions(-) >>> >>> diff --git a/print-tree.c b/print-tree.c >>> index 3c585e31f1fc..687f871db302 100644 >>> --- a/print-tree.c >>> +++ b/print-tree.c >>> @@ -803,14 +803,25 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key) >>> } >>> } >>> >>> -static void print_uuid_item(struct extent_buffer *l, unsigned long offset, >>> - u32 item_size) >>> +static void print_uuid_item(struct extent_buffer *l, int slot, >>> + unsigned long offset, u32 item_size) >>> { >>> + struct btrfs_key key; >>> + char uuid_str[BTRFS_UUID_UNPARSED_SIZE]; >>> + u8 uuid[BTRFS_UUID_SIZE]; >>> + >>> + /* Reassemble the uuid from key.objecitd and key.offset */ >>> + btrfs_item_key_to_cpu(l, &key, slot); >>> + put_unaligned_le64(key.objectid, uuid); >>> + put_unaligned_le64(key.offset, uuid + sizeof(u64)); >> >> I don't think this will work on a BE system. Because >> btrfs_item_key_to_cpu take the LE representation on-disk and turns it >> into a cpu representation which might very well be BE. And then you >> essentially reverse it by using put_unaligned_le64 for x86 it works fine >> due to it being a LE system. > > Ok, so looking at one of your other patches and some digging seems to > indicate that btrfs explicitly generates LE uuids so your code is > correct, however it's not obvoious from this patch itself. I suggest to > put either a comment above the put_unaligned or a statement in the > commit message that uuids are always generated in little-endian format Or just skip the key endian converting. Since it's byte order to byte order, just memcpy() disk key, with proper comment seems cleaner. BTW UUID doesn't get affected by endian. Because UUID is not a u128 value, but just 16 bytes, like checksum. In csum case, we just use memcpy() and write_extent_buffer() without doing any converting. Thanks, Qu > >> >> >>> + uuid_unparse(uuid, uuid_str); >>> + >>> if (item_size & (sizeof(u64) - 1)) { >>> printf("btrfs: uuid item with illegal size %lu!\n", >>> (unsigned long)item_size); >>> return; >>> } >>> + printf("\t\tuuid %s\n", uuid_str); >>> while (item_size) { >>> __le64 subvol_id; >>> >>> @@ -1297,7 +1308,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb) >>> break; >>> case BTRFS_UUID_KEY_SUBVOL: >>> case BTRFS_UUID_KEY_RECEIVED_SUBVOL: >>> - print_uuid_item(eb, btrfs_item_ptr_offset(eb, i), >>> + print_uuid_item(eb, i, btrfs_item_ptr_offset(eb, i), >>> btrfs_item_size_nr(eb, i)); >>> break; >>> case BTRFS_STRING_ITEM_KEY: { >>> >> -- >> 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 >> > -- > 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 >
On 31.10.2017 09:35, Qu Wenruo wrote: > > > On 2017年10月31日 15:29, Nikolay Borisov wrote: >> >> >> On 31.10.2017 09:15, Nikolay Borisov wrote: >>> >>> >>> On 31.10.2017 06:03, Qu Wenruo wrote: >>>> For key type BTRFS_UUID_KEY_SUBVOL or BTRFS_UUID_KEY_RECEIVED_SUBVOL the >>>> key objectid and key offset are just half of the UUID. >>>> >>>> However we just print the key as %llu, which is converted from little >>>> endian, not byte order for UUID, nor the traditional 36 bytes human >>>> readable uuid format. >>>> >>>> Although true engineer can easily convert it in their brain, but to >>>> make it easier for search, output the result UUID using the 36 chars format. >>>> >>>> Cc: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> >>>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>>> --- >>>> Inspired by UUID related work from Misono. >>>> --- >>>> print-tree.c | 17 ++++++++++++++--- >>>> 1 file changed, 14 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/print-tree.c b/print-tree.c >>>> index 3c585e31f1fc..687f871db302 100644 >>>> --- a/print-tree.c >>>> +++ b/print-tree.c >>>> @@ -803,14 +803,25 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key) >>>> } >>>> } >>>> >>>> -static void print_uuid_item(struct extent_buffer *l, unsigned long offset, >>>> - u32 item_size) >>>> +static void print_uuid_item(struct extent_buffer *l, int slot, >>>> + unsigned long offset, u32 item_size) >>>> { >>>> + struct btrfs_key key; >>>> + char uuid_str[BTRFS_UUID_UNPARSED_SIZE]; >>>> + u8 uuid[BTRFS_UUID_SIZE]; >>>> + >>>> + /* Reassemble the uuid from key.objecitd and key.offset */ >>>> + btrfs_item_key_to_cpu(l, &key, slot); >>>> + put_unaligned_le64(key.objectid, uuid); >>>> + put_unaligned_le64(key.offset, uuid + sizeof(u64)); >>> >>> I don't think this will work on a BE system. Because >>> btrfs_item_key_to_cpu take the LE representation on-disk and turns it >>> into a cpu representation which might very well be BE. And then you >>> essentially reverse it by using put_unaligned_le64 for x86 it works fine >>> due to it being a LE system. >> >> Ok, so looking at one of your other patches and some digging seems to >> indicate that btrfs explicitly generates LE uuids so your code is >> correct, however it's not obvoious from this patch itself. I suggest to >> put either a comment above the put_unaligned or a statement in the >> commit message that uuids are always generated in little-endian format > > Or just skip the key endian converting. > > Since it's byte order to byte order, just memcpy() disk key, with proper > comment seems cleaner. > > BTW UUID doesn't get affected by endian. Because UUID is not a u128 > value, but just 16 bytes, like checksum. > In csum case, we just use memcpy() and write_extent_buffer() without > doing any converting. It does get affected, for more info you can check out commit f9727a17db9b ("uuid: rename uuid types"). It seems after that little endian types really refer to guid as per the commit message and the the "one true UUID" is actually BE. Btrfs apparently chose to use little endian since the on-disk format uses that. Given this, I do think that an explicit statement that btrfs' uuids are LE is necessary. > > Thanks, > Qu > >> >>> >>> >>>> + uuid_unparse(uuid, uuid_str); >>>> + >>>> if (item_size & (sizeof(u64) - 1)) { >>>> printf("btrfs: uuid item with illegal size %lu!\n", >>>> (unsigned long)item_size); >>>> return; >>>> } >>>> + printf("\t\tuuid %s\n", uuid_str); >>>> while (item_size) { >>>> __le64 subvol_id; >>>> >>>> @@ -1297,7 +1308,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb) >>>> break; >>>> case BTRFS_UUID_KEY_SUBVOL: >>>> case BTRFS_UUID_KEY_RECEIVED_SUBVOL: >>>> - print_uuid_item(eb, btrfs_item_ptr_offset(eb, i), >>>> + print_uuid_item(eb, i, btrfs_item_ptr_offset(eb, i), >>>> btrfs_item_size_nr(eb, i)); >>>> break; >>>> case BTRFS_STRING_ITEM_KEY: { >>>> >>> -- >>> 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 >>> >> -- >> 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 >> > -- 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
On 2017年10月31日 15:41, Nikolay Borisov wrote: > > > On 31.10.2017 09:35, Qu Wenruo wrote: >> >> >> On 2017年10月31日 15:29, Nikolay Borisov wrote: >>> >>> >>> On 31.10.2017 09:15, Nikolay Borisov wrote: >>>> >>>> >>>> On 31.10.2017 06:03, Qu Wenruo wrote: >>>>> For key type BTRFS_UUID_KEY_SUBVOL or BTRFS_UUID_KEY_RECEIVED_SUBVOL the >>>>> key objectid and key offset are just half of the UUID. >>>>> >>>>> However we just print the key as %llu, which is converted from little >>>>> endian, not byte order for UUID, nor the traditional 36 bytes human >>>>> readable uuid format. >>>>> >>>>> Although true engineer can easily convert it in their brain, but to >>>>> make it easier for search, output the result UUID using the 36 chars format. >>>>> >>>>> Cc: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> >>>>> Signed-off-by: Qu Wenruo <wqu@suse.com> >>>>> --- >>>>> Inspired by UUID related work from Misono. >>>>> --- >>>>> print-tree.c | 17 ++++++++++++++--- >>>>> 1 file changed, 14 insertions(+), 3 deletions(-) >>>>> >>>>> diff --git a/print-tree.c b/print-tree.c >>>>> index 3c585e31f1fc..687f871db302 100644 >>>>> --- a/print-tree.c >>>>> +++ b/print-tree.c >>>>> @@ -803,14 +803,25 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key) >>>>> } >>>>> } >>>>> >>>>> -static void print_uuid_item(struct extent_buffer *l, unsigned long offset, >>>>> - u32 item_size) >>>>> +static void print_uuid_item(struct extent_buffer *l, int slot, >>>>> + unsigned long offset, u32 item_size) >>>>> { >>>>> + struct btrfs_key key; >>>>> + char uuid_str[BTRFS_UUID_UNPARSED_SIZE]; >>>>> + u8 uuid[BTRFS_UUID_SIZE]; >>>>> + >>>>> + /* Reassemble the uuid from key.objecitd and key.offset */ >>>>> + btrfs_item_key_to_cpu(l, &key, slot); >>>>> + put_unaligned_le64(key.objectid, uuid); >>>>> + put_unaligned_le64(key.offset, uuid + sizeof(u64)); >>>> >>>> I don't think this will work on a BE system. Because >>>> btrfs_item_key_to_cpu take the LE representation on-disk and turns it >>>> into a cpu representation which might very well be BE. And then you >>>> essentially reverse it by using put_unaligned_le64 for x86 it works fine >>>> due to it being a LE system. >>> >>> Ok, so looking at one of your other patches and some digging seems to >>> indicate that btrfs explicitly generates LE uuids so your code is >>> correct, however it's not obvoious from this patch itself. I suggest to >>> put either a comment above the put_unaligned or a statement in the >>> commit message that uuids are always generated in little-endian format >> >> Or just skip the key endian converting. >> >> Since it's byte order to byte order, just memcpy() disk key, with proper >> comment seems cleaner. >> >> BTW UUID doesn't get affected by endian. Because UUID is not a u128 >> value, but just 16 bytes, like checksum. >> In csum case, we just use memcpy() and write_extent_buffer() without >> doing any converting. > > It does get affected, for more info you can check out commit > f9727a17db9b ("uuid: rename uuid types"). Indeed, true UUID is not u8[16], but u32 + u16 + u16 + u16 + u48, so it's affected by endian. (Well, things in practice sometimes get different from its original/formal design) Anyway, I'll extract the key to uuid convert to btrfs_key_to_uuid(), and add comment in that function so we don't need to bother this tricky part any longer. Thanks for the review, Qu > It seems after that little > endian types really refer to guid as per the commit message and the the > "one true UUID" is actually BE. Btrfs apparently chose to use little > endian since the on-disk format uses that. Given this, I do think that > an explicit statement that btrfs' uuids are LE is necessary. > > >> >> Thanks, >> Qu >> >>> >>>> >>>> >>>>> + uuid_unparse(uuid, uuid_str); >>>>> + >>>>> if (item_size & (sizeof(u64) - 1)) { >>>>> printf("btrfs: uuid item with illegal size %lu!\n", >>>>> (unsigned long)item_size); >>>>> return; >>>>> } >>>>> + printf("\t\tuuid %s\n", uuid_str); >>>>> while (item_size) { >>>>> __le64 subvol_id; >>>>> >>>>> @@ -1297,7 +1308,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb) >>>>> break; >>>>> case BTRFS_UUID_KEY_SUBVOL: >>>>> case BTRFS_UUID_KEY_RECEIVED_SUBVOL: >>>>> - print_uuid_item(eb, btrfs_item_ptr_offset(eb, i), >>>>> + print_uuid_item(eb, i, btrfs_item_ptr_offset(eb, i), >>>>> btrfs_item_size_nr(eb, i)); >>>>> break; >>>>> case BTRFS_STRING_ITEM_KEY: { >>>>> >>>> -- >>>> 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 >>>> >>> -- >>> 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 --git a/print-tree.c b/print-tree.c index 3c585e31f1fc..687f871db302 100644 --- a/print-tree.c +++ b/print-tree.c @@ -803,14 +803,25 @@ void btrfs_print_key(struct btrfs_disk_key *disk_key) } } -static void print_uuid_item(struct extent_buffer *l, unsigned long offset, - u32 item_size) +static void print_uuid_item(struct extent_buffer *l, int slot, + unsigned long offset, u32 item_size) { + struct btrfs_key key; + char uuid_str[BTRFS_UUID_UNPARSED_SIZE]; + u8 uuid[BTRFS_UUID_SIZE]; + + /* Reassemble the uuid from key.objecitd and key.offset */ + btrfs_item_key_to_cpu(l, &key, slot); + put_unaligned_le64(key.objectid, uuid); + put_unaligned_le64(key.offset, uuid + sizeof(u64)); + uuid_unparse(uuid, uuid_str); + if (item_size & (sizeof(u64) - 1)) { printf("btrfs: uuid item with illegal size %lu!\n", (unsigned long)item_size); return; } + printf("\t\tuuid %s\n", uuid_str); while (item_size) { __le64 subvol_id; @@ -1297,7 +1308,7 @@ void btrfs_print_leaf(struct btrfs_root *root, struct extent_buffer *eb) break; case BTRFS_UUID_KEY_SUBVOL: case BTRFS_UUID_KEY_RECEIVED_SUBVOL: - print_uuid_item(eb, btrfs_item_ptr_offset(eb, i), + print_uuid_item(eb, i, btrfs_item_ptr_offset(eb, i), btrfs_item_size_nr(eb, i)); break; case BTRFS_STRING_ITEM_KEY: {
For key type BTRFS_UUID_KEY_SUBVOL or BTRFS_UUID_KEY_RECEIVED_SUBVOL the key objectid and key offset are just half of the UUID. However we just print the key as %llu, which is converted from little endian, not byte order for UUID, nor the traditional 36 bytes human readable uuid format. Although true engineer can easily convert it in their brain, but to make it easier for search, output the result UUID using the 36 chars format. Cc: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> Signed-off-by: Qu Wenruo <wqu@suse.com> --- Inspired by UUID related work from Misono. --- print-tree.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)