Bug#824895: [PATCH] warn against likely data loss
On 5 September 2016 at 17:35, Nicholas D Steeveswrote: > On Tue, Jul 26, 2016 at 12:35:56PM +0100, Dimitri John Ledkov wrote: >> On 20 May 2016 at 22:48, Nicholas D Steeves wrote: >> > >> > Because btrfs-convert is known to fail, and is known to causes >> > data-loss that may take some months to manifest, we must provide a >> > warning, because data-loss is serious. This patch also includes a >> > contrib initramfs hook which is installed to >> > /usr/share/doc/btrfs-progs; it must be manually copied into place; >> > I've included it because this is the first place a sophisticated user >> > would look. >> > >> >> Adding a btrfs-convert initramfs hook (albeit disabled) and warning >> not to use it, sounds a lot like negative suggestion encouraging >> people to use it. >> >> I do not want to ship btrfs-convert (nor provide suggestions) in the >> initramfs precisely because upstream discourages people from using it. >> >> I also fail to see how this is an RC bug. btrfs-convert is a crutch >> for people to toy and try out btrfs, and always has been. > > Primary issue: btrfs-convert provides no warning > Secondary issue introduced by proposed fix: btrfs-convert is a diversion > script > > To solve the primary issue, would you prefer a quilt patch to the source? > The problem of encouraging masochistic users is only relevant to the > workaround for the issue a diversion script creates. > > Raid5/6 profiles are also now on the list of features upstream considers > dangerous, but there is yet no warning. > Please submit warnings as patches upstream -- Regards, Dimitri.
Bug#824895: [PATCH] warn against likely data loss
On Tue, Jul 26, 2016 at 12:35:56PM +0100, Dimitri John Ledkov wrote: > On 20 May 2016 at 22:48, Nicholas D Steeveswrote: > > > > Because btrfs-convert is known to fail, and is known to causes > > data-loss that may take some months to manifest, we must provide a > > warning, because data-loss is serious. This patch also includes a > > contrib initramfs hook which is installed to > > /usr/share/doc/btrfs-progs; it must be manually copied into place; > > I've included it because this is the first place a sophisticated user > > would look. > > > > Adding a btrfs-convert initramfs hook (albeit disabled) and warning > not to use it, sounds a lot like negative suggestion encouraging > people to use it. > > I do not want to ship btrfs-convert (nor provide suggestions) in the > initramfs precisely because upstream discourages people from using it. > > I also fail to see how this is an RC bug. btrfs-convert is a crutch > for people to toy and try out btrfs, and always has been. Primary issue: btrfs-convert provides no warning Secondary issue introduced by proposed fix: btrfs-convert is a diversion script To solve the primary issue, would you prefer a quilt patch to the source? The problem of encouraging masochistic users is only relevant to the workaround for the issue a diversion script creates. Raid5/6 profiles are also now on the list of features upstream considers dangerous, but there is yet no warning. Cheers, Nicholas
Bug#824895: [PATCH] warn against likely data loss
Control: severity -1 wishlist Control: tag -1 wontfix Hello, On 20 May 2016 at 22:48, Nicholas D Steeveswrote: > Package: btrfs-progs > Version: 4.5.2-1 > Tags: patch > Control: severity -1 severe > > Because btrfs-convert is known to fail, and is known to causes > data-loss that may take some months to manifest, we must provide a > warning, because data-loss is serious. This patch also includes a > contrib initramfs hook which is installed to > /usr/share/doc/btrfs-progs; it must be manually copied into place; > I've included it because this is the first place a sophisticated user > would look. > > Signed-off-by: Nicholas D Steeves Adding a btrfs-convert initramfs hook (albeit disabled) and warning not to use it, sounds a lot like negative suggestion encouraging people to use it. I do not want to ship btrfs-convert (nor provide suggestions) in the initramfs precisely because upstream discourages people from using it. I also fail to see how this is an RC bug. btrfs-convert is a crutch for people to toy and try out btrfs, and always has been. -- Regards, Dimitri.
Bug#824895: [PATCH] warn against likely data loss
Justification: Problems like these [1] still exist. [1] https://www.spinics.net/lists/linux-btrfs/msg56596.html
Bug#824895: [PATCH] warn against likely data loss
Package: btrfs-progs Version: 4.5.2-1 Tags: patch Control: severity -1 severe Because btrfs-convert is known to fail, and is known to causes data-loss that may take some months to manifest, we must provide a warning, because data-loss is serious. This patch also includes a contrib initramfs hook which is installed to /usr/share/doc/btrfs-progs; it must be manually copied into place; I've included it because this is the first place a sophisticated user would look. Signed-off-by: Nicholas D Steevesdiff -ur --new-file btrfs-progs-4.5.2/debian/local/btrfs-convert.hook btrfs-progs-4.5.2.changed/debian/local/btrfs-convert.hook --- btrfs-progs-4.5.2/debian/local/btrfs-convert.hook 1969-12-31 19:00:00.0 -0500 +++ btrfs-progs-4.5.2.changed/debian/local/btrfs-convert.hook 2016-05-20 17:00:42.460132609 -0400 @@ -0,0 +1,8 @@ +#!/bin/sh + +. /usr/share/initramfs-tools/hook-functions + +if command -v /bin/btrfs-convert >/dev/null 2>&1; then +copy_exec /bin/btrfs-convert bin/btrfs-convert +copy_exec /bin/btrfs-convert.real bin/btrfs-convert.real +fi diff -ur --new-file btrfs-progs-4.5.2/debian/local/btrfs-convert.wrapper btrfs-progs-4.5.2.changed/debian/local/btrfs-convert.wrapper --- btrfs-progs-4.5.2/debian/local/btrfs-convert.wrapper 1969-12-31 19:00:00.0 -0500 +++ btrfs-progs-4.5.2.changed/debian/local/btrfs-convert.wrapper 2016-05-20 17:01:01.084103049 -0400 @@ -0,0 +1,35 @@ +#!/bin/sh + +cat << !EOF! + WARNING +Btrfs-convert is known to fail. An upstream rewrite of the +utility is currently in progress. At this point in time, +btrfs-convert is provided for experimental purposes only. + +Please do not convert filesystems filled with data you value. +Expect that you will need to restore from backup. Beware, +the dataloss may not be immediately apparent, and may take +some time to manifest. + +If you choose to experiment with btrfs-convert, please do so +with the willingness to work with linux-bt...@vger.kernel.org + +!EOF! +echo -n 'Proceed (Yes/No)?: ' + +while true; do +read -r choice +case "$choice" in +Yes ) +exec /bin/btrfs-convert.real "$@" +exit $? +;; +no | n | No | NO ) +echo "Thank you for making the safe choice" +exit 0 +;; +* ) +echo -n "Please answer Yes or No: " +;; +esac +done diff -ur --new-file btrfs-progs-4.5.2/debian/rules btrfs-progs-4.5.2.changed/debian/rules --- btrfs-progs-4.5.2/debian/rules 2016-05-10 05:15:55.0 -0400 +++ btrfs-progs-4.5.2.changed/debian/rules 2016-05-20 16:59:16.708269112 -0400 @@ -27,6 +27,12 @@ # Adding initramfs-tools integration install -D -m 0755 debian/local/btrfs.hook debian/btrfs-progs/usr/share/initramfs-tools/hooks/btrfs install -D -m 0755 debian/local/btrfs.local-premount debian/btrfs-progs/usr/share/initramfs-tools/scripts/local-premount/btrfs + install -D -m 0755 debian/local/btrfs-convert.hook debian/btrfs-progs/usr/share/doc/btrfs-convert.hook + +# divert btrfs-convert to btrfs-convert.real +# and use a wrapper script to notify it is dangerous + mv debian/btrfs-progs/bin/btrfs-convert debian/btrfs-progs/bin/btrfs-convert.real + install -D -m 0755 debian/local/btrfs-convert.wrapper debian/btrfs-progs/bin/btrfs-convert # Needs autopkgtest override_dh_auto_test: