Re: [PATCH 1/2] btrfs-progs: convert: Prevent bit overflow for cctx->total_bytes

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2930 bytes --]



On 2020/7/29 上午9:56, Neal Gompa wrote:
> On Tue, Jul 28, 2020 at 9:20 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>
>>
>>
>> On 2020/7/28 下午9:14, Neal Gompa wrote:
>>> On Thu, Jul 23, 2020 at 8:02 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>>>>
>>>>
>>>>
>>>> On 2020/7/23 下午9:31, Neal Gompa wrote:
>>>>> On Wed, Jul 22, 2020 at 7:33 AM David Sterba <dsterba@suse.cz> wrote:
>>>>>>
>>>>>> On Wed, Jul 22, 2020 at 06:58:39AM +0800, Qu Wenruo wrote:
>>>>>>>>> Thus casting both would definitely be right, without the need to refer
>>>>>>>>> to the complex rule book, thus save the reviewer several minutes.
>>>>>>>>
>>>>>>>> The opposite, if you send me code that's not following known schemes or
>>>>>>>> idiomatic schemes I'll be highly suspicious and looking for the reasons
>>>>>>>> why it's that way and making sure it's correct costs way more time.
>>>>>>>>
>>>>>>> OK, then would you please remove one casting at merge time, or do I need
>>>>>>> to resend?
>>>>>>
>>>>>> Yeah, I fix such things routinely no need to resend.
>>>>>
>>>>> I have a report[1] that seems to look like this patch solves it, is
>>>>> that correct?
>>>>>
>>>>> [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1851674#c7
>>>>>
>>>> Yep, looks like the same bug.
>>>>
>>>
>>> So I backported this fix into btrfs-progs-5.7-4.fc32[1], and the
>>> reporter is still seeing issues[2].
>>>
>>> Pasting from the bug comment[2]:
>>>
>>> [liveuser@localhost-live ~]$ sudo btrfs-convert /dev/sda2
>>> create btrfs filesystem:
>>>     blocksize: 4096
>>>     nodesize:  16384
>>>     features:  extref, skinny-metadata (default)
>>>     checksum:  crc32c
>>> creating ext2 image file
>>> creating btrfs metadata
>>> convert/source-ext2.c:845: ext2_copy_inodes: BUG_ON `ret` triggered, value -28
>>
>> This means we have no space left.
>>
>> We don't know if it's the fs already exhausted (little space left for
>> EXT*), or it's btrfs' extent allocator not working.
>>
>> Would it possible to update the image?
>>
> 
> I'm not sure what you're asking here? Do you mean update the live
> environment? You can boot a Fedora 32 live environment and update the
> btrfs-progs package before using it like the bug reporter did...

I mean, upload the binary dump or e2image dump of /dev/sda2.
e2image -Q would be much smaller and without any data stored in it.

Sometimes that's the easiest way to debug.
Or we need to add tons of probes to btrfs-convert and let the reporter
to try-and-report to pin down the bug.

> 
>> BTW, even btrfs-convert crashed, the fs should be completely fine, just
>> as nothing happened to it (from the point of view of ext*)
>>
> 
> I figured as much, but we shouldn't have a case where btrfs-convert
> crashes like this...

Yep, the BUG_ON() should be removed and replaced with a more graceful exit.

I'll do that soon.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]