linux-ext4 - Re: [PATCH 0/3] Fiemap, an extent mapping ioctl

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 10 Sep 2008 10:07:19 -0400
From:	Theodore Tso <tytso@....EDU>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Mark Fasheh <mfasheh@...e.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andreas Dilger <adilger@....com>,
	Eric Sandeen <esandeen@...hat.com>, linux-ext4@...r.kernel.org
Subject: Re: [PATCH 0/3] Fiemap, an extent mapping ioctl

On Wed, Sep 10, 2008 at 09:47:27AM -0400, Christoph Hellwig wrote:
> On Wed, Sep 10, 2008 at 05:49:34AM -0700, Mark Fasheh wrote:
> > * FIEMAP_FLAG_XATTR
> > If this flag is set, the extents returned will describe the inodes
> > extended attribute lookup tree, instead of it's data tree.
> 
> So does this actually make sense for any filesystem but XFS?  Still
> seems like a not too useful option for a highlevel generic interface.

Yes, Andreas has submitted a proposed extension which would make this
make sense for ext4.

> > 	__u32	fe_device;   /* device number for extent */
> 
> As sayd before please kill thise one.  It doesn't make any sense at all
> for any merged or near mainline FS.  It'd be an utterly stupid
> lustre-specific hack that still doesn't make much sense.

As a suggestion, how about adding some __u32 and __u64 reserved
fields, some of which are reserved for use by the filesystem (i.e.,
you have to check f_type as returned by statfs to know what can be
used), and some for future generic expansions to the generic FIEMAP
interface.  When designing a userspace interfaces, leaving room in
data structures for future expansion is a good thing, since it can
avoid needing to have multiple versions of an interface when we need
to expand the data structure.

While I can understand not wanting to have any new kernel functions
without any mainline users for that interface, leaving a few reserved
fields in data structures is just a smart thing to do, and has little
or no downsides.

Whether or not we explicitly define bitfields like
FIEMAP_EXTENT_SECONDARY is much less of an issue, since we can always
grab a bit position later without changing the size of the interface.
But if it turns out we want to add something like some way of
identifying which physical device is used by for a particular extent
--- something which btrfs will need, by the way --- not having room in
the data structure will just bite us later.  So why not reserve some
room now?

Interface minimalism to allow for flexibility later is one thing; but
taken to extremes, it it's just stupid.  For example, if we didn't
have any filesystems that needed 64-bit fields in mainline, would that
be a justification for making data structures to use 32-bit fields and
forcing a flag data interface change in the future.  Sometimes we can
look ahead just a little bit and design interfaces which are foward
compatible.  And it's pretty clear that btrfs will need something like
fe_device, although whether it should be a 32-bit index or something
else like a 128-bit UUID admittedly might not be clear at the moment.
But leaving room in the data structure for future growth is just a
intelligent thing to do.

Regards,

					- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html