All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ted Ts'o" <tytso@mit.edu>
To: kreijack@inwind.it
Cc: linux-btrfs <linux-btrfs@vger.kernel.org>,
	Liu Bo <liubo2009@cn.fujitsu.com>
Subject: Re: Btrfs and data nocow per inode basis
Date: Tue, 12 Jun 2012 14:32:09 -0400	[thread overview]
Message-ID: <20120612183209.GC1803@thunk.org> (raw)
In-Reply-To: <4FD77F45.3060000@libero.it>

On Tue, Jun 12, 2012 at 07:41:25PM +0200, Goffredo Baroncelli wrote:
> 
> After a bit of googling I found a Liu Bo patches which add the ability
> to set the NOCOW flags to a btrfs file.[1]
> 
> However it seems that it was not present in the current (v1.42.3)
> e2fsprogs suite.
> 
> There is any reason which stopped the adoption of this patch ?

It's a textbook example of how *not* to try to get a patch into
e2fsprogs.

1) It's a huge patch that makes a much larger set of changes than what
is necessary to achieve the desired results (the EXT2_*_FL to
FS_*_FL flags is hugely gratuitous)

2) Because of all of the noise in the patch, something that was
completely missed was that the patch did ***NOT*** allow the setting
of the NOCOW flag.

My apologies for not having the time send a reply back.  It fell
between the cracks.

> Does make sense to add the chattr/lsattr capability to the btrfs tool (I
> am thinking about new commands like "btrfs filesystem chattr"/"btrfs
> filesystem lsattr") ?

The lsattr/chattr commands and the flags were always originally been
defined for the ext2/3/4 file systems.  It was the reiserfs filesystem
that just glommed onto that interface and hijacked the flags
definition into a file system independent interface.  At this point
enough other file systems have used those ioctl's and the flags
interface that I *do* try to coordinate with other file systems.

This means that I coordinate flag assignments and try not to use flag
bits in ext2/3/4 that have been made visible by other file systems
(even though the 32-bit flag space is getting pretty crowded, and
there's not much space left).  And I will add support for flags into
lsattr/chattr that are not supported by ext2/3/4.

But a massive change which tries to do a global rename of EXT2_*_FL to
FS_*_FL in e2fsprogs for no good reason?  That just adds code churn
and it makes it harder to validate that the patch is correct, for no
good user-visible benefit.

Regards,

					- Ted

  reply	other threads:[~2012-06-12 18:32 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-10  6:47 Btrfs and data nocow for per inode basis Goffredo Baroncelli
2012-06-12 17:41 ` Btrfs and data nocow " Goffredo Baroncelli
2012-06-12 18:32   ` Ted Ts'o [this message]
2012-06-12 19:15     ` Ted Ts'o
2012-06-12 20:44       ` Chris Mason
2012-06-12 21:02         ` Goffredo Baroncelli
2012-06-12 21:10         ` Ted Ts'o
2012-06-12 22:19           ` Ted Ts'o
2012-06-13  7:42           ` Liu Bo
2012-06-12 22:08         ` David Sterba

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=20120612183209.GC1803@thunk.org \
    --to=tytso@mit.edu \
    --cc=kreijack@inwind.it \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=liubo2009@cn.fujitsu.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 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.