Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Fri, Aug 9, 2013 at 1:01 AM, David Sterba wrote: > On Wed, Aug 07, 2013 at 12:33:09PM +0100, Filipe David Manana wrote: >> Thanks, I missed to find that before. >> The implementation is very different from the one I proposed. > > That's one of the fundaental questions how to store the information: > inside existing structures, via xattrs, under new tree items. Each one > has pros and cons. > >> > Designing and merging the properties feature takes time, but we want to >> > tune >> > simple things now. The wiki project mentions ‘tune2fs’ as an example, but >> > the >> > project details are not always accurate about how to do the things, it’s >> > more >> > like ideas what to do. If you’re going to work on that, please claim the >> > project on the wiki, and possibly write more details abou the design. >> >> I will. > > The project is titled as persistent mount options, are you willing to > take the more general "per-object properties" task? IMHO there's not > much difference, the UI should be the same, just that it implements > per-fs or per-subvolume properties like mount options. The rest of the > object properties has to be collected and agreed on. I'm sure there's > community knowledge of what's desired, so it's a matter of writing it > down and bikeshe^Wagreement on the naming syntax. Yes, I will. I'll get back to this soon and update the wiki page. thanks > > david -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Wed, Aug 07, 2013 at 12:33:09PM +0100, Filipe David Manana wrote: > Thanks, I missed to find that before. > The implementation is very different from the one I proposed. That's one of the fundaental questions how to store the information: inside existing structures, via xattrs, under new tree items. Each one has pros and cons. > > Designing and merging the properties feature takes time, but we want to tune > > simple things now. The wiki project mentions ‘tune2fs’ as an example, but > > the > > project details are not always accurate about how to do the things, it’s > > more > > like ideas what to do. If you’re going to work on that, please claim the > > project on the wiki, and possibly write more details abou the design. > > I will. The project is titled as persistent mount options, are you willing to take the more general "per-object properties" task? IMHO there's not much difference, the UI should be the same, just that it implements per-fs or per-subvolume properties like mount options. The rest of the object properties has to be collected and agreed on. I'm sure there's community knowledge of what's desired, so it's a matter of writing it down and bikeshe^Wagreement on the naming syntax. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Wed, Aug 07, 2013 at 03:46:20PM +0200, Martin Steigerwald wrote: > > Because really, the motivation sounds like it's primarily for significant > > on-disk format changes controlled by mount options. I understand that > > motivation more than being able to persist something like "noatime." > > For a hotplug-able SSD having noatime stored persistently IMHO makes a lot of > sense as well. I agree, and we can let btrfs understand noatime (or ro) even if they get processed by vfs layer. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
Am Dienstag, 6. August 2013, 16:05:50 schrieb Eric Sandeen: > On 8/6/13 3:45 PM, Filipe David Manana wrote: > > On Tue, Aug 6, 2013 at 9:37 PM, Eric Sandeen wrote: > >> On 8/6/13 1:27 PM, Filipe David Borba Manana wrote: > >>> This change allows for most mount options to be persisted in > >>> the filesystem, and be applied when the filesystem is mounted. > >>> If the same options are specified at mount time, the persisted > >>> values for those options are ignored. > >>> > >>> The only options not supported are: subvol, subvolid, subvolrootid, > >>> device and thread_pool. This limitation is due to how this feature > >>> is implemented: basically there's an optional value (of type > >>> struct btrfs_dir_item) in the tree of tree roots used to store the > >>> list of options in the same format as they are passed to btrfs_mount(). > >>> This means any mount option that takes effect before the tree of tree > >>> roots is setup is not supported. > >>> > >>> To set these options, the user space tool btrfstune was modified > >>> to persist the list of options into an unmounted filesystem's > >>> tree of tree roots. > >> > >> So, it does this thing, ok - but why? > >> What is seen as the administrative advantage of this new mechanism? > >> > >> Just to play devil's advocate, and to add a bit of history: > >> > >> On any production system, the filesystems will be mounted via fstab, > >> which has the advantages of being widely known, well understood, and > >> 100% expected - as well as being transparent, unsurprising, and seamless. > >> > >> For history: ext4 did this too. And now it's in a situation where it's > >> got mount options coming at it from both the superblock and from > >> the commandline (or fstab), and sometimes they conflict; it also tries > >> to report mount options in /proc/mounts, but has grown hairy code > >> to decide which ones to print and which ones to not print (if it's > >> a "default" option, don't print it in /proc/mounts, but what's default, > >> code-default or fs-default?) And it's really kind of an ugly mess. > >> > >> Further, mounting 2 filesystems w/ no options in fstab or on the > >> commandline, and getting different behavior due to hidden (sorry, > >> persistent) options in the fs itself is surprising, and surprise > >> is rarely good. > >> > >> So this patch adds 100+ lines of new code, to implement this idea, but: > >> what is the advantage? Unless there is a compelling administrative > >> use case, I'd vote against it. Lines of code that don't exist don't > >> have bugs. ;) > > > > There was a recent good example (imho at least) mentioned by Xavier > > Gnata some time ago: > > > > http://comments.gmane.org/gmane.comp.file-systems.btrfs/26011 > > > > cheers > > Hm, I see. I forgot about hotplugging in my "most systems mount > via fstab" assertion. :) > > I was thinking (and Josef just suggested too) that making a > dir flag, saying "everything under this dir gets compressed" might make > more sense for that scenario than adding a whole slew of > on-disk-persistent-mount-option code. > > Because really, the motivation sounds like it's primarily for significant > on-disk format changes controlled by mount options. I understand that > motivation more than being able to persist something like "noatime." For a hotplug-able SSD having noatime stored persistently IMHO makes a lot of sense as well. I won´t be surprised that at some time, extern SSDs or extern $successor-of- SSD will be replacing extern harddisks. -- Martin 'Helios' Steigerwald - http://www.Lichtvoll.de GPG: 03B0 0D6C 0040 0710 4AFA B82F 991B EAAC A599 84C7 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Wed, Aug 7, 2013 at 11:40 AM, David Sterba wrote: > On Tue, Aug 06, 2013 at 07:27:20PM +0100, Filipe David Borba Manana wrote: >> This change allows for most mount options to be persisted in >> the filesystem, and be applied when the filesystem is mounted. >> If the same options are specified at mount time, the persisted >> values for those options are ignored. >> >> The only options not supported are: subvol, subvolid, subvolrootid, >> device and thread_pool. This limitation is due to how this feature >> is implemented: basically there's an optional value (of type >> struct btrfs_dir_item) in the tree of tree roots used to store the >> list of options in the same format as they are passed to btrfs_mount(). >> This means any mount option that takes effect before the tree of tree >> roots is setup is not supported. >> >> To set these options, the user space tool btrfstune was modified >> to persist the list of options into an unmounted filesystem's >> tree of tree roots. > > We’ve seen this proposed in the past, IIRC this thread contains most of what > has been discussed: > http://thread.gmane.org/gmane.comp.file-systems.btrfs/19757 Thanks, I missed to find that before. The implementation is very different from the one I proposed. > > Implementing just whole-filesystem mount options is not convering all > usecases, > more broad approach like per-subvolume or per-file settings is desired. There > are always settings that aren’t known now but would be useful in the future, > so > we need a flexible infrastructure to maintain the properties. > > There was a proposed implementation for set/get properties: > http://thread.gmane.org/gmane.comp.file-systems.btrfs/18287 Will take a detailed look at it. Thanks for pointing it out David. Why was it never picked? > > From the implementation side, I very much want to abandon the separate > btrfstune utility. Currently it’s a bandaid because there’s nothing better > atm. > > Designing and merging the properties feature takes time, but we want to tune > simple things now. The wiki project mentions ‘tune2fs’ as an example, but the > project details are not always accurate about how to do the things, it’s more > like ideas what to do. If you’re going to work on that, please claim the > project on the wiki, and possibly write more details abou the design. I will. > > david -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Wed, Aug 7, 2013 at 11:48 AM, David Sterba wrote: > On Tue, Aug 06, 2013 at 04:05:50PM -0500, Eric Sandeen wrote: >> I was thinking (and Josef just suggested too) that making a >> dir flag, saying "everything under this dir gets compressed" might make >> more sense for that scenario than adding a whole slew of >> on-disk-persistent-mount-option code. > > We have this dir flag stored in the attributes, chattr +c dir/, but this > cannot be tuned further - no way to specify the compression algorithm > (or for example the target ratio as compression hint), or we can't say > "never compress files in this dir (even if mounted with compression)" > etc. What Eric/Josef suggested, I think I've implemented it in the following RFC: https://patchwork.kernel.org/patch/2840235/ The only thing it doesn't permit, is to disallow compression for individual files or files placed under specific directories. Any idea how would this be specified? (not via chattr I guess) > > david -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Tue, Aug 06, 2013 at 04:05:50PM -0500, Eric Sandeen wrote: > I was thinking (and Josef just suggested too) that making a > dir flag, saying "everything under this dir gets compressed" might make > more sense for that scenario than adding a whole slew of > on-disk-persistent-mount-option code. We have this dir flag stored in the attributes, chattr +c dir/, but this cannot be tuned further - no way to specify the compression algorithm (or for example the target ratio as compression hint), or we can't say "never compress files in this dir (even if mounted with compression)" etc. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Tue, Aug 06, 2013 at 07:27:20PM +0100, Filipe David Borba Manana wrote: > This change allows for most mount options to be persisted in > the filesystem, and be applied when the filesystem is mounted. > If the same options are specified at mount time, the persisted > values for those options are ignored. > > The only options not supported are: subvol, subvolid, subvolrootid, > device and thread_pool. This limitation is due to how this feature > is implemented: basically there's an optional value (of type > struct btrfs_dir_item) in the tree of tree roots used to store the > list of options in the same format as they are passed to btrfs_mount(). > This means any mount option that takes effect before the tree of tree > roots is setup is not supported. > > To set these options, the user space tool btrfstune was modified > to persist the list of options into an unmounted filesystem's > tree of tree roots. We’ve seen this proposed in the past, IIRC this thread contains most of what has been discussed: http://thread.gmane.org/gmane.comp.file-systems.btrfs/19757 Implementing just whole-filesystem mount options is not convering all usecases, more broad approach like per-subvolume or per-file settings is desired. There are always settings that aren’t known now but would be useful in the future, so we need a flexible infrastructure to maintain the properties. There was a proposed implementation for set/get properties: http://thread.gmane.org/gmane.comp.file-systems.btrfs/18287 >From the implementation side, I very much want to abandon the separate btrfstune utility. Currently it’s a bandaid because there’s nothing better atm. Designing and merging the properties feature takes time, but we want to tune simple things now. The wiki project mentions ‘tune2fs’ as an example, but the project details are not always accurate about how to do the things, it’s more like ideas what to do. If you’re going to work on that, please claim the project on the wiki, and possibly write more details abou the design. david -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Wed, Aug 7, 2013 at 4:04 AM, Eric Sandeen wrote: > On 8/6/13 1:27 PM, Filipe David Borba Manana wrote: >> This change allows for most mount options to be persisted in >> the filesystem, and be applied when the filesystem is mounted. >> If the same options are specified at mount time, the persisted >> values for those options are ignored. > > I thought the plan was to make commandline mount options > override persistent ones, but that doesn't seem to be the case, > at least in this example: Yes, that was the idea. However I didn't try all possible combinations of mount and persistent parameters. > > # ./btrfstune -o compress,discard,ssd /dev/sdb1 > New persistent options: compress,discard,ssd > # mount -o nossd /dev/sdb1 /mnt/test > # dmesg | tail > [ 995.657233] btrfs: use ssd allocation scheme > [ 995.661501] btrfs: disk space caching is enabled Yes. Misses some checks like the ones I added for the space_cache / no_space_cache combinations: + if (token == Opt_no_space_cache && +parsed[Opt_space_cache]) + continue; + if (token == Opt_space_cache && +parsed[Opt_no_space_cache]) + continue; > > and /proc/mounts is similarly confused, showing both options: > > # grep sdb1 /proc/mounts > /dev/sdb1 /mnt/test btrfs > rw,seclabel,relatime,compress=zlib,nossd,ssd,discard,space_cache 0 0 > > (This is the trail of woe I was talking about from ext4-land) ;) > >> The only options not supported are: subvol, subvolid, subvolrootid, >> device and thread_pool. This limitation is due to how this feature >> is implemented: basically there's an optional value (of type >> struct btrfs_dir_item) in the tree of tree roots used to store the >> list of options in the same format as they are passed to btrfs_mount(). >> This means any mount option that takes effect before the tree of tree >> roots is setup is not supported. > > Just FWIW, vfs-level mount options are also not supported; i.e. "noatime" > or "ro" won't work either, because the code is already past the VFS > option parsing: > > # ./btrfstune -o compress,discard,ro /dev/sdb1 > Current persistent options: compress,discard > New persistent options: compress,discard,ro > # mount /dev/sdb1 /mnt/test > mount: wrong fs type, bad option, bad superblock on /dev/sdb1, ... > # dmesg | tail > [ 817.681417] btrfs: unrecognized mount option 'ro' > [ 817.694689] btrfs: open_ctree failed Yes, that would be a next step to work on after getting community feedback about the approach (from a user point of view and the technical details / implementation). Thanks for trying it and for your feedback Eric. > >> To set these options, the user space tool btrfstune was modified >> to persist the list of options into an unmounted filesystem's >> tree of tree roots. > > If this goes forward, you'd want an easy way to display > them, not just set them, I suppose. > > Thanks, > -Eric > >> NOTE: Like the corresponding btrfs-progs patch, this is a WIP with >> the goal o gathering feedback. >> >> Signed-off-by: Filipe David Borba Manana > > -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Tue, Aug 6, 2013 at 10:05 PM, Eric Sandeen wrote: > On 8/6/13 3:45 PM, Filipe David Manana wrote: >> On Tue, Aug 6, 2013 at 9:37 PM, Eric Sandeen wrote: >>> On 8/6/13 1:27 PM, Filipe David Borba Manana wrote: This change allows for most mount options to be persisted in the filesystem, and be applied when the filesystem is mounted. If the same options are specified at mount time, the persisted values for those options are ignored. The only options not supported are: subvol, subvolid, subvolrootid, device and thread_pool. This limitation is due to how this feature is implemented: basically there's an optional value (of type struct btrfs_dir_item) in the tree of tree roots used to store the list of options in the same format as they are passed to btrfs_mount(). This means any mount option that takes effect before the tree of tree roots is setup is not supported. To set these options, the user space tool btrfstune was modified to persist the list of options into an unmounted filesystem's tree of tree roots. >>> >>> So, it does this thing, ok - but why? >>> What is seen as the administrative advantage of this new mechanism? >>> >>> Just to play devil's advocate, and to add a bit of history: >>> >>> On any production system, the filesystems will be mounted via fstab, >>> which has the advantages of being widely known, well understood, and >>> 100% expected - as well as being transparent, unsurprising, and seamless. >>> >>> For history: ext4 did this too. And now it's in a situation where it's >>> got mount options coming at it from both the superblock and from >>> the commandline (or fstab), and sometimes they conflict; it also tries >>> to report mount options in /proc/mounts, but has grown hairy code >>> to decide which ones to print and which ones to not print (if it's >>> a "default" option, don't print it in /proc/mounts, but what's default, >>> code-default or fs-default?) And it's really kind of an ugly mess. >>> >>> Further, mounting 2 filesystems w/ no options in fstab or on the >>> commandline, and getting different behavior due to hidden (sorry, >>> persistent) options in the fs itself is surprising, and surprise >>> is rarely good. >>> >>> So this patch adds 100+ lines of new code, to implement this idea, but: >>> what is the advantage? Unless there is a compelling administrative >>> use case, I'd vote against it. Lines of code that don't exist don't >>> have bugs. ;) >> >> There was a recent good example (imho at least) mentioned by Xavier >> Gnata some time ago: >> >> http://comments.gmane.org/gmane.comp.file-systems.btrfs/26011 >> >> cheers > > Hm, I see. I forgot about hotplugging in my "most systems mount > via fstab" assertion. :) > > I was thinking (and Josef just suggested too) that making a > dir flag, saying "everything under this dir gets compressed" might make > more sense for that scenario than adding a whole slew of > on-disk-persistent-mount-option code. I like that idea too. Thanks for the suggestion. A quick experiment allowed for that approach in under 20 lines, will test it a bit more. > > Because really, the motivation sounds like it's primarily for significant > on-disk format changes controlled by mount options. I understand that > motivation more than being able to persist something like "noatime." > > -Eric > -- Filipe David Manana, "Reasonable men adapt themselves to the world. Unreasonable men adapt the world to themselves. That's why all progress depends on unreasonable men." -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On 8/6/13 1:27 PM, Filipe David Borba Manana wrote: > This change allows for most mount options to be persisted in > the filesystem, and be applied when the filesystem is mounted. > If the same options are specified at mount time, the persisted > values for those options are ignored. I thought the plan was to make commandline mount options override persistent ones, but that doesn't seem to be the case, at least in this example: # ./btrfstune -o compress,discard,ssd /dev/sdb1 New persistent options: compress,discard,ssd # mount -o nossd /dev/sdb1 /mnt/test # dmesg | tail [ 995.657233] btrfs: use ssd allocation scheme [ 995.661501] btrfs: disk space caching is enabled and /proc/mounts is similarly confused, showing both options: # grep sdb1 /proc/mounts /dev/sdb1 /mnt/test btrfs rw,seclabel,relatime,compress=zlib,nossd,ssd,discard,space_cache 0 0 (This is the trail of woe I was talking about from ext4-land) ;) > The only options not supported are: subvol, subvolid, subvolrootid, > device and thread_pool. This limitation is due to how this feature > is implemented: basically there's an optional value (of type > struct btrfs_dir_item) in the tree of tree roots used to store the > list of options in the same format as they are passed to btrfs_mount(). > This means any mount option that takes effect before the tree of tree > roots is setup is not supported. Just FWIW, vfs-level mount options are also not supported; i.e. "noatime" or "ro" won't work either, because the code is already past the VFS option parsing: # ./btrfstune -o compress,discard,ro /dev/sdb1 Current persistent options: compress,discard New persistent options: compress,discard,ro # mount /dev/sdb1 /mnt/test mount: wrong fs type, bad option, bad superblock on /dev/sdb1, ... # dmesg | tail [ 817.681417] btrfs: unrecognized mount option 'ro' [ 817.694689] btrfs: open_ctree failed > To set these options, the user space tool btrfstune was modified > to persist the list of options into an unmounted filesystem's > tree of tree roots. If this goes forward, you'd want an easy way to display them, not just set them, I suppose. Thanks, -Eric > NOTE: Like the corresponding btrfs-progs patch, this is a WIP with > the goal o gathering feedback. > > Signed-off-by: Filipe David Borba Manana -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On 8/6/13 8:20 PM, Duncan wrote: > Eric Sandeen posted on Tue, 06 Aug 2013 15:37:30 -0500 as excerpted: > >> On 8/6/13 1:27 PM, Filipe David Borba Manana wrote: >>> This change allows for most mount options to be persisted in the >>> filesystem, and be applied when the filesystem is mounted. >>> If the same options are specified at mount time, the persisted values >>> for those options are ignored. >>> >>> The only options not supported are: subvol, subvolid, subvolrootid, >>> device and thread_pool. This limitation is due to how this feature is >>> implemented: basically there's an optional value (of type struct >>> btrfs_dir_item) in the tree of tree roots used to store the list of >>> options in the same format as they are passed to btrfs_mount(). >>> This means any mount option that takes effect before the tree of tree >>> roots is setup is not supported. >>> >>> To set these options, the user space tool btrfstune was modified to >>> persist the list of options into an unmounted filesystem's tree of tree >>> roots. >> >> So, it does this thing, ok - but why? >> What is seen as the administrative advantage of this new mechanism? >> >> Just to play devil's advocate, and to add a bit of history: >> >> On any production system, the filesystems will be mounted via fstab, >> which has the advantages of being widely known, well understood, and >> 100% expected - as well as being transparent, unsurprising, and >> seamless. >> >> For history: ext4 did this too. And now it's in a situation where it's >> got mount options coming at it from both the superblock and from the >> commandline (or fstab), and sometimes they conflict; it also tries to >> report mount options in /proc/mounts, but has grown hairy code to decide >> which ones to print and which ones to not print (if it's a "default" >> option, don't print it in /proc/mounts, but what's default, code-default >> or fs-default?) And it's really kind of an ugly mess. >> >> Further, mounting 2 filesystems w/ no options in fstab or on the >> commandline, and getting different behavior due to hidden (sorry, >> persistent) options in the fs itself is surprising, and surprise is >> rarely good. >> >> So this patch adds 100+ lines of new code, to implement this idea, but: >> what is the advantage? Unless there is a compelling administrative use >> case, I'd vote against it. Lines of code that don't exist don't have >> bugs. ;) > > As an admin, there's some options I want always applied as site policy. > Here, that includes compress=lzo, autodefrag and noatime. And with all But as an admin, you can add that to fstab, right? > the capacities btrfs has what with raid and the like, particularly if > someone's needing to use device= (which won't go in the persistent > options for what should be pretty obvious reasons) a bunch of times, that > fstab line can get quite long indeed![1] > > Just like the kernel has a config option for a built-in commandline that > takes some of the pressure off the actually passed commandline for > options that are pretty much always going to be used so it's easier to > administer because only the possibly dynamic options or those going > against the builtin commandline defaults need passed, a filesystem as > complex and multi-optioned as btrfs is, really NEEDS some way to persist > the options that are effectively site policy default, so the admin > doesn't have to worry about them any longer. Again, fstab is perfectly sufficient, simply for site policy. It seems that your main argument here (no pun intended) is that the sheer length of the options string becomes unwieldy. i.e. "it's too long for fstab, so it must be moved into the filesystem." "too long" is a bit subjective, unless util-linux has an actual string limit. If not, I guess it's mostly aesthetics. > FWIW, I had assumed persistent mount options were planned as a given and > simply hadn't been implemented yet. Because to me it's a no-brainer. > After all, people don't have to use the feature if they don't like it. no, but we still have to maintain it ;) So just speaking for myself, 100+ new lines of code forever after, vs. "I find my fstab to be unattractive" is an obvious choice. > And it sure saves a headache when what might otherwise be a dozen > parameters passed in can be cut in half or better, leaving only the ones > that are going to differ depending on circumstances to worry about. I > know that from experience with the kernel builtin commandline option! But you still have to set them all. Is it less headache to use btrfstune vs edit fstab? I'm not really convinced. I guess the argument that it's easier to notice specific differences against a site-wide (hidden) set of options, vs. a bunch of long option strings resonates somewhat. *shrug* well, I did my ghost-of-christmas-future bit; it's just my $0.02. Thanks, -Eric > --- > [1] I ran initr*less root-on-md-raid for many years. isn't that kind of doing it wrong, though? i
Re: [PATCH RFC] Btrfs: add support for persistent mount options
Eric Sandeen posted on Tue, 06 Aug 2013 15:37:30 -0500 as excerpted: > On 8/6/13 1:27 PM, Filipe David Borba Manana wrote: >> This change allows for most mount options to be persisted in the >> filesystem, and be applied when the filesystem is mounted. >> If the same options are specified at mount time, the persisted values >> for those options are ignored. >> >> The only options not supported are: subvol, subvolid, subvolrootid, >> device and thread_pool. This limitation is due to how this feature is >> implemented: basically there's an optional value (of type struct >> btrfs_dir_item) in the tree of tree roots used to store the list of >> options in the same format as they are passed to btrfs_mount(). >> This means any mount option that takes effect before the tree of tree >> roots is setup is not supported. >> >> To set these options, the user space tool btrfstune was modified to >> persist the list of options into an unmounted filesystem's tree of tree >> roots. > > So, it does this thing, ok - but why? > What is seen as the administrative advantage of this new mechanism? > > Just to play devil's advocate, and to add a bit of history: > > On any production system, the filesystems will be mounted via fstab, > which has the advantages of being widely known, well understood, and > 100% expected - as well as being transparent, unsurprising, and > seamless. > > For history: ext4 did this too. And now it's in a situation where it's > got mount options coming at it from both the superblock and from the > commandline (or fstab), and sometimes they conflict; it also tries to > report mount options in /proc/mounts, but has grown hairy code to decide > which ones to print and which ones to not print (if it's a "default" > option, don't print it in /proc/mounts, but what's default, code-default > or fs-default?) And it's really kind of an ugly mess. > > Further, mounting 2 filesystems w/ no options in fstab or on the > commandline, and getting different behavior due to hidden (sorry, > persistent) options in the fs itself is surprising, and surprise is > rarely good. > > So this patch adds 100+ lines of new code, to implement this idea, but: > what is the advantage? Unless there is a compelling administrative use > case, I'd vote against it. Lines of code that don't exist don't have > bugs. ;) As an admin, there's some options I want always applied as site policy. Here, that includes compress=lzo, autodefrag and noatime. And with all the capacities btrfs has what with raid and the like, particularly if someone's needing to use device= (which won't go in the persistent options for what should be pretty obvious reasons) a bunch of times, that fstab line can get quite long indeed![1] Just like the kernel has a config option for a built-in commandline that takes some of the pressure off the actually passed commandline for options that are pretty much always going to be used so it's easier to administer because only the possibly dynamic options or those going against the builtin commandline defaults need passed, a filesystem as complex and multi-optioned as btrfs is, really NEEDS some way to persist the options that are effectively site policy default, so the admin doesn't have to worry about them any longer. FWIW, I had assumed persistent mount options were planned as a given and simply hadn't been implemented yet. Because to me it's a no-brainer. After all, people don't have to use the feature if they don't like it. And it sure saves a headache when what might otherwise be a dozen parameters passed in can be cut in half or better, leaving only the ones that are going to differ depending on circumstances to worry about. I know that from experience with the kernel builtin commandline option! --- [1] I ran initr*less root-on-md-raid for many years. That involved passing a complicated set of md1=/dev/sda3,/dev/sdb3,/dev/sdc3,/dev/sdd3 type options to the kernel, along with more usual options I was passing, and I was SO happy when the kernel got that built-in-commandline option and I could put the default set in there, such that I only had to worry about passing that parameter for the backup boot option, thus along with several other passed options I was able to put in the builtin, shrinking the actual passed kernel commandline dramatically, so only the stuff that wasn't the default needed passed for a particular boot option. It would sure be nice to be able to do the same thing, but at the filesystem level, here! -- Duncan - List replies preferred. No HTML msgs. "Every nonfree program has a lord, a master -- and if you use the program, he is your master." Richard Stallman -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On 8/6/13 3:45 PM, Filipe David Manana wrote: > On Tue, Aug 6, 2013 at 9:37 PM, Eric Sandeen wrote: >> On 8/6/13 1:27 PM, Filipe David Borba Manana wrote: >>> This change allows for most mount options to be persisted in >>> the filesystem, and be applied when the filesystem is mounted. >>> If the same options are specified at mount time, the persisted >>> values for those options are ignored. >>> >>> The only options not supported are: subvol, subvolid, subvolrootid, >>> device and thread_pool. This limitation is due to how this feature >>> is implemented: basically there's an optional value (of type >>> struct btrfs_dir_item) in the tree of tree roots used to store the >>> list of options in the same format as they are passed to btrfs_mount(). >>> This means any mount option that takes effect before the tree of tree >>> roots is setup is not supported. >>> >>> To set these options, the user space tool btrfstune was modified >>> to persist the list of options into an unmounted filesystem's >>> tree of tree roots. >> >> So, it does this thing, ok - but why? >> What is seen as the administrative advantage of this new mechanism? >> >> Just to play devil's advocate, and to add a bit of history: >> >> On any production system, the filesystems will be mounted via fstab, >> which has the advantages of being widely known, well understood, and >> 100% expected - as well as being transparent, unsurprising, and seamless. >> >> For history: ext4 did this too. And now it's in a situation where it's >> got mount options coming at it from both the superblock and from >> the commandline (or fstab), and sometimes they conflict; it also tries >> to report mount options in /proc/mounts, but has grown hairy code >> to decide which ones to print and which ones to not print (if it's >> a "default" option, don't print it in /proc/mounts, but what's default, >> code-default or fs-default?) And it's really kind of an ugly mess. >> >> Further, mounting 2 filesystems w/ no options in fstab or on the >> commandline, and getting different behavior due to hidden (sorry, >> persistent) options in the fs itself is surprising, and surprise >> is rarely good. >> >> So this patch adds 100+ lines of new code, to implement this idea, but: >> what is the advantage? Unless there is a compelling administrative >> use case, I'd vote against it. Lines of code that don't exist don't >> have bugs. ;) > > There was a recent good example (imho at least) mentioned by Xavier > Gnata some time ago: > > http://comments.gmane.org/gmane.comp.file-systems.btrfs/26011 > > cheers Hm, I see. I forgot about hotplugging in my "most systems mount via fstab" assertion. :) I was thinking (and Josef just suggested too) that making a dir flag, saying "everything under this dir gets compressed" might make more sense for that scenario than adding a whole slew of on-disk-persistent-mount-option code. Because really, the motivation sounds like it's primarily for significant on-disk format changes controlled by mount options. I understand that motivation more than being able to persist something like "noatime." -Eric -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On Tue, Aug 6, 2013 at 9:37 PM, Eric Sandeen wrote:
> On 8/6/13 1:27 PM, Filipe David Borba Manana wrote:
>> This change allows for most mount options to be persisted in
>> the filesystem, and be applied when the filesystem is mounted.
>> If the same options are specified at mount time, the persisted
>> values for those options are ignored.
>>
>> The only options not supported are: subvol, subvolid, subvolrootid,
>> device and thread_pool. This limitation is due to how this feature
>> is implemented: basically there's an optional value (of type
>> struct btrfs_dir_item) in the tree of tree roots used to store the
>> list of options in the same format as they are passed to btrfs_mount().
>> This means any mount option that takes effect before the tree of tree
>> roots is setup is not supported.
>>
>> To set these options, the user space tool btrfstune was modified
>> to persist the list of options into an unmounted filesystem's
>> tree of tree roots.
>
> So, it does this thing, ok - but why?
> What is seen as the administrative advantage of this new mechanism?
>
> Just to play devil's advocate, and to add a bit of history:
>
> On any production system, the filesystems will be mounted via fstab,
> which has the advantages of being widely known, well understood, and
> 100% expected - as well as being transparent, unsurprising, and seamless.
>
> For history: ext4 did this too. And now it's in a situation where it's
> got mount options coming at it from both the superblock and from
> the commandline (or fstab), and sometimes they conflict; it also tries
> to report mount options in /proc/mounts, but has grown hairy code
> to decide which ones to print and which ones to not print (if it's
> a "default" option, don't print it in /proc/mounts, but what's default,
> code-default or fs-default?) And it's really kind of an ugly mess.
>
> Further, mounting 2 filesystems w/ no options in fstab or on the
> commandline, and getting different behavior due to hidden (sorry,
> persistent) options in the fs itself is surprising, and surprise
> is rarely good.
>
> So this patch adds 100+ lines of new code, to implement this idea, but:
> what is the advantage? Unless there is a compelling administrative
> use case, I'd vote against it. Lines of code that don't exist don't
> have bugs. ;)
There was a recent good example (imho at least) mentioned by Xavier
Gnata some time ago:
http://comments.gmane.org/gmane.comp.file-systems.btrfs/26011
cheers
>
> -Eric
>
>> NOTE: Like the corresponding btrfs-progs patch, this is a WIP with
>> the goal o gathering feedback.
>>
>> Signed-off-by: Filipe David Borba Manana
>> ---
>> fs/btrfs/ctree.h | 11 +++-
>> fs/btrfs/disk-io.c | 72
>> +++-
>> fs/btrfs/super.c | 46 +++--
>> 3 files changed, 125 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
>> index cbb1263..24363df 100644
>> --- a/fs/btrfs/ctree.h
>> +++ b/fs/btrfs/ctree.h
>> @@ -121,6 +121,14 @@ struct btrfs_ordered_sum;
>> */
>> #define BTRFS_FREE_INO_OBJECTID -12ULL
>>
>> +/*
>> + * Item that stores permanent mount options. These options
>> + * have effect if they are not specified as well at mount
>> + * time (that is, if a permanent option is also specified at
>> + * mount time, the later wins).
>> + */
>> +#define BTRFS_PERSISTENT_OPTIONS_OBJECTID -13ULL
>> +
>> /* dummy objectid represents multiple objectids */
>> #define BTRFS_MULTIPLE_OBJECTIDS -255ULL
>>
>> @@ -3739,7 +3747,8 @@ void btrfs_exit_sysfs(void);
>> ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size);
>>
>> /* super.c */
>> -int btrfs_parse_options(struct btrfs_root *root, char *options);
>> +int btrfs_parse_options(struct btrfs_root *root, char *options,
>> + int parsing_persistent, int **options_parsed);
>> int btrfs_sync_fs(struct super_block *sb, int wait);
>>
>> #ifdef CONFIG_PRINTK
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 254cdc8..eeabdd4 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -2077,6 +2077,53 @@ static void del_fs_roots(struct btrfs_fs_info
>> *fs_info)
>> }
>> }
>>
>> +static char *get_persistent_options(struct btrfs_root *tree_root)
>> +{
>> + int ret;
>> + struct btrfs_key key;
>> + struct btrfs_path *path;
>> + struct extent_buffer *leaf;
>> + struct btrfs_dir_item *di;
>> + u32 name_len, data_len;
>> + char *options = NULL;
>> +
>> + path = btrfs_alloc_path();
>> + if (!path)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + key.objectid = BTRFS_PERSISTENT_OPTIONS_OBJECTID;
>> + key.type = 0;
>> + key.offset = 0;
>> +
>> + ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
>> + if (ret < 0)
>> + goto out;
>> + if (ret > 0) {
>> + ret = 0;
>> + goto out;
>> + }
>> +
>> + leaf = path->nodes[0];
>>
Re: [PATCH RFC] Btrfs: add support for persistent mount options
On 8/6/13 1:27 PM, Filipe David Borba Manana wrote:
> This change allows for most mount options to be persisted in
> the filesystem, and be applied when the filesystem is mounted.
> If the same options are specified at mount time, the persisted
> values for those options are ignored.
>
> The only options not supported are: subvol, subvolid, subvolrootid,
> device and thread_pool. This limitation is due to how this feature
> is implemented: basically there's an optional value (of type
> struct btrfs_dir_item) in the tree of tree roots used to store the
> list of options in the same format as they are passed to btrfs_mount().
> This means any mount option that takes effect before the tree of tree
> roots is setup is not supported.
>
> To set these options, the user space tool btrfstune was modified
> to persist the list of options into an unmounted filesystem's
> tree of tree roots.
So, it does this thing, ok - but why?
What is seen as the administrative advantage of this new mechanism?
Just to play devil's advocate, and to add a bit of history:
On any production system, the filesystems will be mounted via fstab,
which has the advantages of being widely known, well understood, and
100% expected - as well as being transparent, unsurprising, and seamless.
For history: ext4 did this too. And now it's in a situation where it's
got mount options coming at it from both the superblock and from
the commandline (or fstab), and sometimes they conflict; it also tries
to report mount options in /proc/mounts, but has grown hairy code
to decide which ones to print and which ones to not print (if it's
a "default" option, don't print it in /proc/mounts, but what's default,
code-default or fs-default?) And it's really kind of an ugly mess.
Further, mounting 2 filesystems w/ no options in fstab or on the
commandline, and getting different behavior due to hidden (sorry,
persistent) options in the fs itself is surprising, and surprise
is rarely good.
So this patch adds 100+ lines of new code, to implement this idea, but:
what is the advantage? Unless there is a compelling administrative
use case, I'd vote against it. Lines of code that don't exist don't
have bugs. ;)
-Eric
> NOTE: Like the corresponding btrfs-progs patch, this is a WIP with
> the goal o gathering feedback.
>
> Signed-off-by: Filipe David Borba Manana
> ---
> fs/btrfs/ctree.h | 11 +++-
> fs/btrfs/disk-io.c | 72
> +++-
> fs/btrfs/super.c | 46 +++--
> 3 files changed, 125 insertions(+), 4 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index cbb1263..24363df 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -121,6 +121,14 @@ struct btrfs_ordered_sum;
> */
> #define BTRFS_FREE_INO_OBJECTID -12ULL
>
> +/*
> + * Item that stores permanent mount options. These options
> + * have effect if they are not specified as well at mount
> + * time (that is, if a permanent option is also specified at
> + * mount time, the later wins).
> + */
> +#define BTRFS_PERSISTENT_OPTIONS_OBJECTID -13ULL
> +
> /* dummy objectid represents multiple objectids */
> #define BTRFS_MULTIPLE_OBJECTIDS -255ULL
>
> @@ -3739,7 +3747,8 @@ void btrfs_exit_sysfs(void);
> ssize_t btrfs_listxattr(struct dentry *dentry, char *buffer, size_t size);
>
> /* super.c */
> -int btrfs_parse_options(struct btrfs_root *root, char *options);
> +int btrfs_parse_options(struct btrfs_root *root, char *options,
> + int parsing_persistent, int **options_parsed);
> int btrfs_sync_fs(struct super_block *sb, int wait);
>
> #ifdef CONFIG_PRINTK
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 254cdc8..eeabdd4 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2077,6 +2077,53 @@ static void del_fs_roots(struct btrfs_fs_info *fs_info)
> }
> }
>
> +static char *get_persistent_options(struct btrfs_root *tree_root)
> +{
> + int ret;
> + struct btrfs_key key;
> + struct btrfs_path *path;
> + struct extent_buffer *leaf;
> + struct btrfs_dir_item *di;
> + u32 name_len, data_len;
> + char *options = NULL;
> +
> + path = btrfs_alloc_path();
> + if (!path)
> + return ERR_PTR(-ENOMEM);
> +
> + key.objectid = BTRFS_PERSISTENT_OPTIONS_OBJECTID;
> + key.type = 0;
> + key.offset = 0;
> +
> + ret = btrfs_search_slot(NULL, tree_root, &key, path, 0, 0);
> + if (ret < 0)
> + goto out;
> + if (ret > 0) {
> + ret = 0;
> + goto out;
> + }
> +
> + leaf = path->nodes[0];
> + di = btrfs_item_ptr(leaf, path->slots[0], struct btrfs_dir_item);
> + name_len = btrfs_dir_name_len(leaf, di);
> + data_len = btrfs_dir_data_len(leaf, di);
> + options = kmalloc(data_len + 1, GFP_NOFS);
> + if (!options) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + read_extent_buffer(leaf, option
