From: Li Zefan <lizf@cn.fujitsu.com>
To: David Sterba <dave@jikos.cz>
Cc: "linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>,
Sage Weil <sage@newdream.net>
Subject: Re: [PATCH 1/2] Btrfs: don't make a file partly checksummed through file clone
Date: Fri, 16 Sep 2011 11:01:08 +0800 [thread overview]
Message-ID: <4E72BBF4.7010705@cn.fujitsu.com> (raw)
In-Reply-To: <20110915111638.GD22205@twin.jikos.cz>
David Sterba wrote:
> On Wed, Sep 14, 2011 at 01:25:21PM +0800, Li Zefan wrote:
>> It's because part of the file is checksummed and the other part is not,
>> and then btrfs will complain checksum is not found when we read the file.
>>
>> Disallow file clone if src and dst file have different checksum flag,
>> so we ensure a file is completely checksummed or unchecksummed.
>
> Your fix prevents the bug, but I don't think it's good to let file clone
> fail without any other message. ret is set to -EINVAL at the time of
> 'goto out_fput', which is fine, but the user has no clue what happened
> or how to fix it.
While I agree with you on this comment..
>
> The nodatasum status is recorded in inode flags and remains like that
> regardless of the 'mount -o nodatasum', persistent and de facto
> unchangable (unless the file is created again with the opposite nodatasum
> mount). Even more, the user has no way to find out nodatasum flag of
> any inode/file (the corresponding FS_NODATASUM_FL is not there).
>
> My suggestion how to fix this:
> 1. add FS_NODATASUM_FL file flag and code to set/get via setflags ioctl
This means we can have a file partly checksummed, which is what we want
to avoid in this patch.
> 2. [this patch to skip cloning in case of nodatasum flag mismatch]
> 3. ... add a printk why it failed
I don't think this is a good idea.
>
> The user then has at least option to drop/add the nodatasum flag for one of
> the. Unfortunatelly this makes file cloning less straightforward.
>
I don't know if Chris has plan on finer-grained checksum (not per file
but per extent), if yes, we can eliminate this constraint in file cloning
in the future.
prev parent reply other threads:[~2011-09-16 3:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-09-14 5:25 [PATCH 1/2] Btrfs: don't make a file partly checksummed through file clone Li Zefan
2011-09-14 5:25 ` [PATCH 2/2] Btrfs: don't change inode flag of the dest clone file Li Zefan
2011-09-15 11:43 ` David Sterba
2011-09-14 12:17 ` [PATCH 1/2] Btrfs: don't make a file partly checksummed through file clone Sage Weil
2011-09-15 11:16 ` David Sterba
2011-09-16 3:01 ` Li Zefan [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=4E72BBF4.7010705@cn.fujitsu.com \
--to=lizf@cn.fujitsu.com \
--cc=dave@jikos.cz \
--cc=linux-btrfs@vger.kernel.org \
--cc=sage@newdream.net \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.