Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

[Mike Fedyk]

On Mon, Nov 29, 2010 at 12:02 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
> (Cc: Sage Weil <sage@newdream.net> for changes in async snapshots)
>
> This patchset adds readonly-snapshots support. You can create a
> readonly snapshot, and you can also set a snapshot readonly/writable
> on the fly.
>
> A few readonly checks are added in setattr, permission, remove_xattr
> and set_xattr callbacks, as well as in some ioctls.
>

Great work!

I have a suggestion on defaults when snapshots are created.  I think
they should default to being read-only and if they are meant to be
read-write a flag can be set at creation time (and changable at a
later time as well of course).

This way user/admin preconceptions of a snapshot being read-only can
be enforced by default, and the exception when you want a read-write
snapshot can be available with a switch at the cli level (and probably
a flag at the ioctl level).

It gives one more natural distinction between a snapshot and a
subvolume at the user conceptual level.

What do you think?

Re: Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

[David Arendt]

On 11/29/10 21:02, Mike Fedyk wrote:
> On Mon, Nov 29, 2010 at 12:02 AM, Li Zefan<lizf@cn.fujitsu.com>  wrote:
>> (Cc: Sage Weil<sage@newdream.net>  for changes in async snapshots)
>>
>> This patchset adds readonly-snapshots support. You can create a
>> readonly snapshot, and you can also set a snapshot readonly/writable
>> on the fly.
>>
>> A few readonly checks are added in setattr, permission, remove_xattr
>> and set_xattr callbacks, as well as in some ioctls.
>>
> Great work!
>
> I have a suggestion on defaults when snapshots are created.  I think
> they should default to being read-only and if they are meant to be
> read-write a flag can be set at creation time (and changable at a
> later time as well of course).
>
> This way user/admin preconceptions of a snapshot being read-only can
> be enforced by default, and the exception when you want a read-write
> snapshot can be available with a switch at the cli level (and probably
> a flag at the ioctl level).
>
> It gives one more natural distinction between a snapshot and a
> subvolume at the user conceptual level.
>
> What do you think?
> --
> 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
Hi,

I completely agree with you. I think lots of people use snapshots for 
backup purposes and these ones shouldn't be writable.

Bye,
David Arendt

Re: Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

[Mike Fedyk]

On Mon, Nov 29, 2010 at 12:41 PM, David Arendt <admin@prnet.org> wrote:
> On 11/29/10 21:02, Mike Fedyk wrote:
>>
>> On Mon, Nov 29, 2010 at 12:02 AM, Li Zefan<lizf@cn.fujitsu.com> =C2=A0=
wrote:
>>>
>>> (Cc: Sage Weil<sage@newdream.net> =C2=A0for changes in async snapsh=
ots)
>>>
>>> This patchset adds readonly-snapshots support. You can create a
>>> readonly snapshot, and you can also set a snapshot readonly/writabl=
e
>>> on the fly.
>>>
>>> A few readonly checks are added in setattr, permission, remove_xatt=
r
>>> and set_xattr callbacks, as well as in some ioctls.
>>>
>> Great work!
>>
>> I have a suggestion on defaults when snapshots are created. =C2=A0I =
think
>> they should default to being read-only and if they are meant to be
>> read-write a flag can be set at creation time (and changable at a
>> later time as well of course).
>>
>> This way user/admin preconceptions of a snapshot being read-only can
>> be enforced by default, and the exception when you want a read-write
>> snapshot can be available with a switch at the cli level (and probab=
ly
>> a flag at the ioctl level).
>>
>> It gives one more natural distinction between a snapshot and a
>> subvolume at the user conceptual level.
>>
>> What do you think?
>>
> I completely agree with you. I think lots of people use snapshots for=
 backup
> purposes and these ones shouldn't be writable.
>
=2E... by default.
--
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

Re: Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

[Andrey Kuzmin]

This may sound excessive as any new concept introduction that late in
development, but readonly/writable snapshots could be further
differentiated by naming the latter clones. This way end-user would
naturally perceive snapsot as read-only PIT fs image, while clone
would naturally refer to (writable) head fork.

Regards,
Andrey




On Tue, Nov 30, 2010 at 12:08 AM, Mike Fedyk <mfedyk@mikefedyk.com> wro=
te:
> On Mon, Nov 29, 2010 at 12:41 PM, David Arendt <admin@prnet.org> wrot=
e:
>> On 11/29/10 21:02, Mike Fedyk wrote:
>>>
>>> On Mon, Nov 29, 2010 at 12:02 AM, Li Zefan<lizf@cn.fujitsu.com> =C2=
=A0wrote:
>>>>
>>>> (Cc: Sage Weil<sage@newdream.net> =C2=A0for changes in async snaps=
hots)
>>>>
>>>> This patchset adds readonly-snapshots support. You can create a
>>>> readonly snapshot, and you can also set a snapshot readonly/writab=
le
>>>> on the fly.
>>>>
>>>> A few readonly checks are added in setattr, permission, remove_xat=
tr
>>>> and set_xattr callbacks, as well as in some ioctls.
>>>>
>>> Great work!
>>>
>>> I have a suggestion on defaults when snapshots are created. =C2=A0I=
 think
>>> they should default to being read-only and if they are meant to be
>>> read-write a flag can be set at creation time (and changable at a
>>> later time as well of course).
>>>
>>> This way user/admin preconceptions of a snapshot being read-only ca=
n
>>> be enforced by default, and the exception when you want a read-writ=
e
>>> snapshot can be available with a switch at the cli level (and proba=
bly
>>> a flag at the ioctl level).
>>>
>>> It gives one more natural distinction between a snapshot and a
>>> subvolume at the user conceptual level.
>>>
>>> What do you think?
>>>
>> I completely agree with you. I think lots of people use snapshots fo=
r backup
>> purposes and these ones shouldn't be writable.
>>
> .... by default.
> --
> 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 =C2=A0http://vger.kernel.org/majordomo-info.ht=
ml
>
--
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

Re: Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

[Mike Fedyk]

On Mon, Nov 29, 2010 at 1:31 PM, Andrey Kuzmin
<andrey.v.kuzmin@gmail.com> wrote:
> This may sound excessive as any new concept introduction that late in
> development, but readonly/writable snapshots could be further
> differentiated by naming the latter clones. This way end-user would
> naturally perceive snapsot as read-only PIT fs image, while clone
> would naturally refer to (writable) head fork.
>

I'm not sure we want to take all of the terminology that zfs uses as
it may also bring the percieved drawbacks as well.  Isn't there some
additional overhead for a zfs clone compared to a snapshot?  I'm not
very familiar with zfs so that's why I ask.

Re: Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

[Andrey Kuzmin]

I'm not sure why zfs came up, they don't own the term :). As to
zfs/overhead topic, I doubt there's any difference between clone and
writable shapshot (there should be none, of course, it's just two
different names for the same concept).

Regards,
Andrey




On Tue, Nov 30, 2010 at 12:43 AM, Mike Fedyk <mfedyk@mikefedyk.com> wro=
te:
> On Mon, Nov 29, 2010 at 1:31 PM, Andrey Kuzmin
> <andrey.v.kuzmin@gmail.com> wrote:
>> This may sound excessive as any new concept introduction that late i=
n
>> development, but readonly/writable snapshots could be further
>> differentiated by naming the latter clones. This way end-user would
>> naturally perceive snapsot as read-only PIT fs image, while clone
>> would naturally refer to (writable) head fork.
>>
>
> I'm not sure we want to take all of the terminology that zfs uses as
> it may also bring the percieved drawbacks as well. =C2=A0Isn't there =
some
> additional overhead for a zfs clone compared to a snapshot? =C2=A0I'm=
 not
> very familiar with zfs so that's why I ask.
>
--
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

Re: Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

[C Anthony Risinger]

On Nov 29, 2010, at 3:48 PM, Andrey Kuzmin <andrey.v.kuzmin@gmail.com>
wrote:

> I'm not sure why zfs came up, they don't own the term :). As to
> zfs/overhead topic, I doubt there's any difference between clone and
> writable shapshot (there should be none, of course, it's just two
> different names for the same concept).
>
> Regards,
> Andrey
>
>
>
>
> On Tue, Nov 30, 2010 at 12:43 AM, Mike Fedyk <mfedyk@mikefedyk.com>
> wrote:
>> On Mon, Nov 29, 2010 at 1:31 PM, Andrey Kuzmin
>> <andrey.v.kuzmin@gmail.com> wrote:
>>> This may sound excessive as any new concept introduction that late
>>> in
>>> development, but readonly/writable snapshots could be further
>>> differentiated by naming the latter clones. This way end-user would
>>> naturally perceive snapsot as read-only PIT fs image, while clone
>>> would naturally refer to (writable) head fork.
>>>
>>
>> I'm not sure we want to take all of the terminology that zfs uses as
>> it may also bring the percieved drawbacks as well.  Isn't there some
>> additional overhead for a zfs clone compared to a snapshot?  I'm not
>> very familiar with zfs so that's why I ask.
>>
> --
> 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

I don't like the idea of readonly by default, or further changes to
terminology, for several reasons:

) readonly by default offers no real enhancement whatsoever other than
breaking _anything_ that's written right now
) btrfs readonly is not even really readonly; as superuser could
simply flip a flag to enable writes, readonly merely prevents
accidental writes or misbehaving apps... ie. protecting you from
yourself
) backups are the simple/obvious use case; I personally use btrfs
heavily for LXC containers, in which case nearly every single snapshot
is intended to be writable -- usually cloning a template into a new
domain
) I also use an initramfs hook to provide system rollbacks, also
writable; the hook also provides multiple versions of the "branch"...
all writable
) adding new terms is not a good idea imo; I've already spewed out
many sentences explaining the difference between subvolumes and
snapshots, ie. that there is none... adding another term only adds to
this problem; they each describe the same thing, but differentiate
based on origin or current state, neither of which actually describe
what it _is_-- a new named pointer to a tree, like a git branch -- a
subvolume.

I think a better solution/compromise would be to leave snapshots
writeable by default, since that's more true to what's happening
internally anyway, but maybe introduce a mount option controlling the
default action for that mount point.

C Anthony [mobile]

Re: Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

[Li Zefan]

C Anthony Risinger wrote:
> On Nov 29, 2010, at 3:48 PM, Andrey Kuzmin <andrey.v.kuzmin@gmail.com>
> wrote:
> 
>> I'm not sure why zfs came up, they don't own the term :). As to
>> zfs/overhead topic, I doubt there's any difference between clone and
>> writable shapshot (there should be none, of course, it's just two
>> different names for the same concept).
>>
>> Regards,
>> Andrey
>>
>>
>>
>>
>> On Tue, Nov 30, 2010 at 12:43 AM, Mike Fedyk <mfedyk@mikefedyk.com>
>> wrote:
>>> On Mon, Nov 29, 2010 at 1:31 PM, Andrey Kuzmin
>>> <andrey.v.kuzmin@gmail.com> wrote:
>>>> This may sound excessive as any new concept introduction that late
>>>> in
>>>> development, but readonly/writable snapshots could be further
>>>> differentiated by naming the latter clones. This way end-user would
>>>> naturally perceive snapsot as read-only PIT fs image, while clone
>>>> would naturally refer to (writable) head fork.
>>>>
>>> I'm not sure we want to take all of the terminology that zfs uses as
>>> it may also bring the percieved drawbacks as well.  Isn't there some
>>> additional overhead for a zfs clone compared to a snapshot?  I'm not
>>> very familiar with zfs so that's why I ask.
>>>
>> --
>> 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
> 
> I don't like the idea of readonly by default, or further changes to
> terminology, for several reasons:
> 

I quite agree with you. LVM2 also defaults to read/write for snapshots.

> ) readonly by default offers no real enhancement whatsoever other than
> breaking _anything_ that's written right now

This was the first thing that came to my mind.

> ) btrfs readonly is not even really readonly; as superuser could
> simply flip a flag to enable writes, readonly merely prevents
> accidental writes or misbehaving apps... ie. protecting you from
> yourself
> ) backups are the simple/obvious use case; I personally use btrfs
> heavily for LXC containers, in which case nearly every single snapshot
> is intended to be writable -- usually cloning a template into a new
> domain
> ) I also use an initramfs hook to provide system rollbacks, also
> writable; the hook also provides multiple versions of the "branch"...
> all writable
> ) adding new terms is not a good idea imo; I've already spewed out
> many sentences explaining the difference between subvolumes and
> snapshots, ie. that there is none... adding another term only adds to
> this problem; they each describe the same thing, but differentiate
> based on origin or current state, neither of which actually describe
> what it _is_-- a new named pointer to a tree, like a git branch -- a
> subvolume.
> 
> I think a better solution/compromise would be to leave snapshots
> writeable by default, since that's more true to what's happening
> internally anyway, but maybe introduce a mount option controlling the
> default action for that mount point.
> 
> C Anthony [mobile]

Re: Default to read-only on snapshot creation and have a flag if snapshot should be writable (was: [PATCH 0/5] btrfs: Readonly snapshots)

[Andrey Kuzmin]

In my opinion, the point is not the default snapshot creation mode but
rather default usage, equals user's expectation.

On 11/30/10, Li Zefan <lizf@cn.fujitsu.com> wrote:
> C Anthony Risinger wrote:
>> On Nov 29, 2010, at 3:48 PM, Andrey Kuzmin <andrey.v.kuzmin@gmail.com>
>> wrote:
>>
>>> I'm not sure why zfs came up, they don't own the term :). As to
>>> zfs/overhead topic, I doubt there's any difference between clone and
>>> writable shapshot (there should be none, of course, it's just two
>>> different names for the same concept).
>>>
>>> Regards,
>>> Andrey
>>>
>>>
>>>
>>>
>>> On Tue, Nov 30, 2010 at 12:43 AM, Mike Fedyk <mfedyk@mikefedyk.com>
>>> wrote:
>>>> On Mon, Nov 29, 2010 at 1:31 PM, Andrey Kuzmin
>>>> <andrey.v.kuzmin@gmail.com> wrote:
>>>>> This may sound excessive as any new concept introduction that late
>>>>> in
>>>>> development, but readonly/writable snapshots could be further
>>>>> differentiated by naming the latter clones. This way end-user would
>>>>> naturally perceive snapsot as read-only PIT fs image, while clone
>>>>> would naturally refer to (writable) head fork.
>>>>>
>>>> I'm not sure we want to take all of the terminology that zfs uses as
>>>> it may also bring the percieved drawbacks as well.  Isn't there some
>>>> additional overhead for a zfs clone compared to a snapshot?  I'm not
>>>> very familiar with zfs so that's why I ask.
>>>>
>>> --
>>> 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
>>
>> I don't like the idea of readonly by default, or further changes to
>> terminology, for several reasons:
>>
>
> I quite agree with you. LVM2 also defaults to read/write for snapshots.
>
>> ) readonly by default offers no real enhancement whatsoever other than
>> breaking _anything_ that's written right now
>
> This was the first thing that came to my mind.
>
>> ) btrfs readonly is not even really readonly; as superuser could
>> simply flip a flag to enable writes, readonly merely prevents
>> accidental writes or misbehaving apps... ie. protecting you from
>> yourself
>> ) backups are the simple/obvious use case; I personally use btrfs
>> heavily for LXC containers, in which case nearly every single snapshot
>> is intended to be writable -- usually cloning a template into a new
>> domain
>> ) I also use an initramfs hook to provide system rollbacks, also
>> writable; the hook also provides multiple versions of the "branch"...
>> all writable
>> ) adding new terms is not a good idea imo; I've already spewed out
>> many sentences explaining the difference between subvolumes and
>> snapshots, ie. that there is none... adding another term only adds to
>> this problem; they each describe the same thing, but differentiate
>> based on origin or current state, neither of which actually describe
>> what it _is_-- a new named pointer to a tree, like a git branch -- a
>> subvolume.
>>
>> I think a better solution/compromise would be to leave snapshots
>> writeable by default, since that's more true to what's happening
>> internally anyway, but maybe introduce a mount option controlling the
>> default action for that mount point.
>>
>> C Anthony [mobile]
>


-- 
Regards,
Andrey