Linux Btrfs filesystem development
 help / color / mirror / Atom feed
From: Qu Wenruo <quwenruo@cn.fujitsu.com>
To: <dsterba@suse.cz>, Qu Wenruo <quwenruo.btrfs@gmx.com>,
	btrfs <linux-btrfs@vger.kernel.org>,
	David Sterba <dsterba@suse.com>
Subject: Re: Ideas for btrfs-convert fix(or rework)
Date: Tue, 17 Nov 2015 08:42:13 +0800	[thread overview]
Message-ID: <564A77E5.1040300@cn.fujitsu.com> (raw)
In-Reply-To: <20151116174636.GA31035@twin.jikos.cz>



David Sterba wrote on 2015/11/16 18:46 +0100:
> On Tue, Nov 10, 2015 at 02:27:41PM +0800, Qu Wenruo wrote:
>> [[FIX IDEA]]
>> 1. Too many patches
>
> Not percieved as a problem as long as the patches are well separated and
> reviewable. If you prefer reasonably many preparatory and trivial
> patches, then be it.
>
>> 2. superblock reserve is d*mning hard, not matter at what timing.
>>      Until I need to codes for superblock reserve, the codes are quite
>>      easy to write.
>>      But when it comes to sb reserve, I'm going to get crazy...
>
> What do you mean by the 'sb reserve'?

Superblock reserved space.
And the first 1Mb reserved space.

IIRC, normal btrfs and convert btrfs doesn't reserve super block space well.
For normal btrfs, it's still possible for device extent covering 
btrfs_sb_offset(), not to mention the converted one, which only uses one sb.

But the good news is, although it needs extra codes(about 5 patches and 
over 500 lines), it shouldn't be a big problem as I'll reserve these 
space at early time.

>
>>      I'll either add tons of codes to the simple chunk allocation to
>>      avoid using SB space and keep all chunks are large enough.
>>      Or I need to implement user-space whole chunk relocation codes.
>>
>>      I miss the old days when sb_bytenr can be specified by user...
>>      Then my work should be quite close to end...
>>
>> 3. Too aggressive rework
>>      I suppose this will not be a fix, but a total rework of the whole
>>      btrfs-convert program.
>>      Maybe only less than 30% old codes will stay, most of them will be
>>      rewritten.
>
> I suppose that the rewrite would also touch generic code, so as far as
> it's cleanups and other refactoring I won't object investing the time to
> that.

Not that much, most of the codes are rework of make_btrfs() to allow it 
to alloc metadata and system chunk at early time.
It's almost 10 patches and over 1000 lines to rework the huge 
make_btrfs() into structuralized make_btrfs_v2().

And will remove some btrfs-convert only hooks like custom_alloc_extent().

As now, btrfs-convert will has a meta and system chunk(not covering 
existing ext* data) after make_btrfs_v2().
And then data chunks will be inserted to cover all ext* data.
So no extra extent allocate routine is needed.

>
>>      No matter how I think the new code will be superior than old one,
>>      I'm pretty sure there will be tons of regression or new bugs for
>>      such huge rework.
>>
>>      But without such work, btrfs-convert will always be a mess and no
>>      real support for balance.
>>
>> So any ideas or advice is welcomed for this rework.
>
> In general, I welcome the efforts to make convert better. The in-place
> conversion is a unique feature and a nice to have. Several people
> mentioned that.  Next, we want to add support more filesystems, eg.
> reiserfs code is ready and needs configure checks and minor updates to
> convert.c.  Basically any filesystem that provides a library API similar
> to what ext2 does can be added. Convert code in a good shape would save
> us trouble.
>
> Expecting the regressions, the test set for the convertors needs to be
> enhanced, the current state is less then ideal. Possibly we can use the
> images from e2fsprogs testsuite.

Nice idea.

I'll try to enhance self test after btrfs-convert rework.

Thanks,
Qu

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

      reply	other threads:[~2015-11-17  0:42 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-10  6:27 Ideas for btrfs-convert fix(or rework) Qu Wenruo
2015-11-10  7:55 ` Roman Mamedov
2015-11-10  8:16   ` Qu Wenruo
2015-11-10  9:08     ` Roman Mamedov
2015-11-10  9:18       ` Qu Wenruo
2015-11-10 10:31         ` Duncan
2015-11-12 10:23           ` Vytautas D
2015-11-12 13:27             ` Austin S Hemmelgarn
2015-11-12 14:09               ` Roman Mamedov
2015-11-12 14:38                 ` Austin S Hemmelgarn
2015-11-13  6:41                   ` Duncan
2015-11-16 17:46 ` David Sterba
2015-11-17  0:42   ` Qu Wenruo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=564A77E5.1040300@cn.fujitsu.com \
    --to=quwenruo@cn.fujitsu.com \
    --cc=dsterba@suse.com \
    --cc=dsterba@suse.cz \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=quwenruo.btrfs@gmx.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox