| Message ID | 1395162718-22586-1-git-send-email-psusi@ubuntu.com |
|---|---|
| State | Rejected, archived |
| Headers | show |
On Mar 18, 2014, at 11:11 AM, Phillip Susi <psusi@ubuntu.com> wrote: > mke2fs displays an interactive y/n prompt for certain situations. > This is not appropriate to do when stdin is not a tty. > > Signed-off-by: Phillip Susi <psusi@ubuntu.com> > --- > misc/util.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/misc/util.c b/misc/util.c > index 92ab79f..c469744 100644 > --- a/misc/util.c > +++ b/misc/util.c > @@ -71,6 +71,8 @@ void proceed_question(void) > char buf[256]; > const char *short_yes = _("yY"); > > + if (!isatty(0)) > + return; This means that mke2fs will silently continue to format whatever accidental device was specified on the command line, instead of stopping and asking the user for confirmation. Instead, it should exit with an error in this case. There is already the "-f" (force) option to override the cases where this prompt is used, so if the user wants to do something unusual they should use that as part of the script driving mke2fs. The only time when I think "force" is required when it shouldn't be is in case of full-device formatting. I don't think this should be considered a problem unless there is already a partition table on the device. Formatting the whole device is desirable to avoid alignment issues on RAID devices, so I would be happy if that restriction was lifted. Cheers, Andreas
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 3/18/2014 2:31 PM, Andreas Dilger wrote: > This means that mke2fs will silently continue to format whatever > accidental device was specified on the command line, instead of > stopping and asking the user for confirmation. Instead, it should > exit with an error in this case. There *is* no user to ask when stdin is connected to /dev/null. This second guessing of the user really goes against the unix way and shouldn't be there in the first place. Mean what you say, and say what you mean. If you must second guess the user and give an interactive prompt just in case they made a mistake, then at least only do so when it looks like there is a user there. There is no reason to fail when called from a script. > There is already the "-f" (force) option to override the cases > where this prompt is used, so if the user wants to do something > unusual they should use that as part of the script driving mke2fs. - --yes-i-meant-what-i-said flags are silly, and make for useless inconsistencies between the different mkfs.* utilities that makes writing scripts calling them annoyingly difficult. > The only time when I think "force" is required when it shouldn't be > is in case of full-device formatting. I don't think this should be > considered a problem unless there is already a partition table on > the device. Formatting the whole device is desirable to avoid > alignment issues on RAID devices, so I would be happy if that > restriction was lifted. The same goes for being run on an image file instead of a device node. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTKJTZAAoJEI5FoCIzSKrwqVcH+wYQkv7lP0Sm2Y52XWbfotAN 5yyO3K0w9FFTscHo2YEZ2JE+QdkYadsyuENM41IChNcduJpZjAh6LbVNdlEU9sUq v97a+TJq1ahyshl4rxakm5kdXJo1cSZc8cWZoDk3O4ChDhRZdY0fv/uu+sfw0etr wYkRNgXB6eNmRSUeIj6rPg3sSKDpNcbcDW3x8cP6Ww+KfLzDtMzEMRkhQpQ13I2P eJLebfSQBmiU7hj5xd+TQHORpvk3VxXk4JG1zsytRPekAtNkTFzqi1Uoo75fcTwh Nmn2SR2rq1ls81xBTlNQ5gZQ5mUz3ZIuthXWuF3mpP1SW/om3SCM+ay3S0/NNxw= =iqNm -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 18 Mar 2014, Phillip Susi wrote: > Date: Tue, 18 Mar 2014 14:47:53 -0400 > From: Phillip Susi <psusi@ubuntu.com> > To: Andreas Dilger <adilger@dilger.ca> > Cc: linux-ext4@vger.kernel.org > Subject: Re: [PATCH] mke2fs: don't interact with a non tty > > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 3/18/2014 2:31 PM, Andreas Dilger wrote: > > This means that mke2fs will silently continue to format whatever > > accidental device was specified on the command line, instead of > > stopping and asking the user for confirmation. Instead, it should > > exit with an error in this case. > > There *is* no user to ask when stdin is connected to /dev/null. This > second guessing of the user really goes against the unix way and > shouldn't be there in the first place. Mean what you say, and say > what you mean. > > If you must second guess the user and give an interactive prompt just > in case they made a mistake, then at least only do so when it looks > like there is a user there. There is no reason to fail when called > from a script. > > > There is already the "-f" (force) option to override the cases > > where this prompt is used, so if the user wants to do something > > unusual they should use that as part of the script driving mke2fs. > > - --yes-i-meant-what-i-said flags are silly, and make for useless > inconsistencies between the different mkfs.* utilities that makes > writing scripts calling them annoyingly difficult. Yes, it is inconsistent especially in the way that mke2fs is proceeding without any problem on the device which already contains a valid file system (or any other) signature. Which I think we should really change. The problem is that this will break scripts for everybody which is bad. So my idea was to implement the signature check and then skip it if we do not have a tty attached. Just to avoid the breakage. However I do not think that we can just blindly ignore the checks we already have in place in the case that there is no user. But I agree that current behaviour is wrong and it should be changed, however I think that we need to change it the other way, the default should be no - do not proceed and exit. Because believe it or not, people make mistakes. > > > The only time when I think "force" is required when it shouldn't be > > is in case of full-device formatting. I don't think this should be > > considered a problem unless there is already a partition table on > > the device. Formatting the whole device is desirable to avoid > > alignment issues on RAID devices, so I would be happy if that > > restriction was lifted. Agreed, but it should not be lifted, but rather changes to check for signatures on the device. The same way as it is done for example in xfs, or btrfs. Thanks! -Lukas > > The same goes for being run on an image file instead of a device node. > > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2.0.17 (MingW32) > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ > > iQEcBAEBAgAGBQJTKJTZAAoJEI5FoCIzSKrwqVcH+wYQkv7lP0Sm2Y52XWbfotAN > 5yyO3K0w9FFTscHo2YEZ2JE+QdkYadsyuENM41IChNcduJpZjAh6LbVNdlEU9sUq > v97a+TJq1ahyshl4rxakm5kdXJo1cSZc8cWZoDk3O4ChDhRZdY0fv/uu+sfw0etr > wYkRNgXB6eNmRSUeIj6rPg3sSKDpNcbcDW3x8cP6Ww+KfLzDtMzEMRkhQpQ13I2P > eJLebfSQBmiU7hj5xd+TQHORpvk3VxXk4JG1zsytRPekAtNkTFzqi1Uoo75fcTwh > Nmn2SR2rq1ls81xBTlNQ5gZQ5mUz3ZIuthXWuF3mpP1SW/om3SCM+ay3S0/NNxw= > =iqNm > -----END PGP SIGNATURE----- > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 3/19/2014 7:26 AM, Lukáš Czerner wrote: > Yes, it is inconsistent especially in the way that mke2fs is > proceeding without any problem on the device which already > contains a valid file system (or any other) signature. Which I > think we should really change. The problem is that this will break > scripts for everybody which is bad. > > So my idea was to implement the signature check and then skip it > if we do not have a tty attached. Just to avoid the breakage. > > However I do not think that we can just blindly ignore the checks > we already have in place in the case that there is no user. But I > agree that current behaviour is wrong and it should be changed, > however I think that we need to change it the other way, the > default should be no - do not proceed and exit. Because believe it > or not, people make mistakes. Then you are right back to breaking scripts. And yes, people make mistakes... and unix *lets* them. You don't see rm stopping every time you try to delete a file and saying really? *That* file? Are you sure? You don't see dd or shell redirection stopping to ask you if you really meant to overwrite that disk or file. There is a *reason* why you are supposed to double check commands you are running as root. And putting a filesystem in an image file is one of the *least* dangerous things you could do. Of all of the things to second guess, and especially to default to "HALT! ERROR!" behavior, this has to be the silliest. If you can't assume that an interactive user knows what they are doing and meant what they said, then at least you should assume that a script writer knows wtf they are doing without asking them to add lots of silly --yes-i-know-what-im-doing-stop-annoying-me flags. > Agreed, but it should not be lifted, but rather changes to check > for signatures on the device. The same way as it is done for > example in xfs, or btrfs. *NO*! Those tools annoy the hell out of me because they do that. I *know* there is another filesystem there already ( as there are on most disks not fresh from the factory ), why do you think I'm telling you to change it? Do what I asked and stop treating me like a child. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTKZ0fAAoJEI5FoCIzSKrwaFQH/RO0NPl2mPAlpaK2YQysegub u80nYpSlpHjiIOLU7RCECakfELIFp1skg7lRsFdL1zLNkor4JkwW8UbOuy75WbS3 +XPAQ/1wxPzsn0J4+QM3PE3X/IZ4NWRMepl0pozpoLine87mL6u6+em2n1r1vsQK HE/1Ma/8jqPPMXPNFDw0LMiYGyAHITfQA4c/FRwlWCbhMt2lG8dsGA7bKl7VCB5D gmkzUF/KbgmY8xnDiIbmSHQbaF+xrIbZl8FGgi4r3CuiGZ2yZBBbs2sTCk6pvIq7 rrTQGwxBuzxaas2h+ZpLfTeFulBlIH2B+ueStinc2Br+TmBeqIKKOQ1+HjO3v6g= =6eGY -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 19 Mar 2014, Phillip Susi wrote: > Date: Wed, 19 Mar 2014 09:35:27 -0400 > From: Phillip Susi <psusi@ubuntu.com> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: Andreas Dilger <adilger@dilger.ca>, linux-ext4@vger.kernel.org > Subject: Re: [PATCH] mke2fs: don't interact with a non tty > > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 3/19/2014 7:26 AM, Lukáš Czerner wrote: > > Yes, it is inconsistent especially in the way that mke2fs is > > proceeding without any problem on the device which already > > contains a valid file system (or any other) signature. Which I > > think we should really change. The problem is that this will break > > scripts for everybody which is bad. > > > > So my idea was to implement the signature check and then skip it > > if we do not have a tty attached. Just to avoid the breakage. > > > > However I do not think that we can just blindly ignore the checks > > we already have in place in the case that there is no user. But I > > agree that current behaviour is wrong and it should be changed, > > however I think that we need to change it the other way, the > > default should be no - do not proceed and exit. Because believe it > > or not, people make mistakes. > > Then you are right back to breaking scripts. And yes, people make > mistakes... and unix *lets* them. You don't see rm stopping every > time you try to delete a file and saying really? *That* file? Are > you sure? You don't see dd or shell redirection stopping to ask you > if you really meant to overwrite that disk or file. There is a > *reason* why you are supposed to double check commands you are running > as root. Then again, many distributions actually do: alias rm='rm -i' and they have reason for it. > > And putting a filesystem in an image file is one of the *least* > dangerous things you could do. Of all of the things to second guess, > and especially to default to "HALT! ERROR!" behavior, this has to be > the silliest. > > If you can't assume that an interactive user knows what they are doing > and meant what they said, then at least you should assume that a > script writer knows wtf they are doing without asking them to add lots > of silly --yes-i-know-what-im-doing-stop-annoying-me flags. > > > Agreed, but it should not be lifted, but rather changes to check > > for signatures on the device. The same way as it is done for > > example in xfs, or btrfs. > > *NO*! Those tools annoy the hell out of me because they do that. I > *know* there is another filesystem there already ( as there are on > most disks not fresh from the factory ), why do you think I'm telling > you to change it? Do what I asked and stop treating me like a child. There was a discussion between me and Dave Chinner a while ago about exactly this topic and he made some very good points. You as a guy working with file system, or file system utilities, testing them and so on will probably see a ton of the warning like that. But like it, or not you're *not* the typical user - you're a developer and there is a huge difference. While you recreate a file system on a device hundreds times in a day, it is not at all typical behaviour. Sysadmins will buy a disk, create a partition table (or not) and create a file system on it - DONE. They would probably never touch the disk again until something breaks. So when they are trying to create a file system on a device that already have a valid signature (file system, lvm, partition ...) it is very likely they made a mistake. And the file system utilities are trying to minimize that by either asking for confirmation (as mke2fs does) or straight up fail if you not force them. So again, the fact that it annoys the hell out of you as a developer is not really a reason to remove this safety net for every sysadmin in the world. You're not the target audience of such measures :) Thanks! -Lukas > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2.0.17 (MingW32) > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ > > iQEcBAEBAgAGBQJTKZ0fAAoJEI5FoCIzSKrwaFQH/RO0NPl2mPAlpaK2YQysegub > u80nYpSlpHjiIOLU7RCECakfELIFp1skg7lRsFdL1zLNkor4JkwW8UbOuy75WbS3 > +XPAQ/1wxPzsn0J4+QM3PE3X/IZ4NWRMepl0pozpoLine87mL6u6+em2n1r1vsQK > HE/1Ma/8jqPPMXPNFDw0LMiYGyAHITfQA4c/FRwlWCbhMt2lG8dsGA7bKl7VCB5D > gmkzUF/KbgmY8xnDiIbmSHQbaF+xrIbZl8FGgi4r3CuiGZ2yZBBbs2sTCk6pvIq7 > rrTQGwxBuzxaas2h+ZpLfTeFulBlIH2B+ueStinc2Br+TmBeqIKKOQ1+HjO3v6g= > =6eGY > -----END PGP SIGNATURE----- >
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 3/19/2014 11:16 AM, Lukáš Czerner wrote: > There was a discussion between me and Dave Chinner a while ago > about exactly this topic and he made some very good points. You as > a guy working with file system, or file system utilities, testing > them and so on will probably see a ton of the warning like that. > But like it, or not you're *not* the typical user - you're a > developer and there is a huge difference. Whether I'm a typical user or a developer does not matter. This is unix, and commands are expected to do what they are told. If you must do such silly things, at the very least you need to not break scripts when doing it, which means if stdin is not a tty, don't assume you are being run by an idiot who needs hand holding and do what you've been told to. Scripts and other tools like say, gparted, should not have to figure out every silly thing you might complain about and add a stupid - --yes-i-really-meant-it flag to every invocation. -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTKb/6AAoJEI5FoCIzSKrwefcIAIHJtyiquS/Zrs4LbCSW1Zoe 5QoLx1ROxqP/Y5CP9mMCcRfsOTXMtZWqf116/Hg5SgIbJ76FGn7peChfPAKw477W HFPWsUmWgdkdVRFJdqny/5MVaOHGd/bD9mt7/8suEwSXX3eAp/gDegKR9Srm+etY n7V/G/uHWQgUQDbs8GAAcpIMA4Y1zcARB19fdNMr1evhDORdN7PAFqxqbixz3gU6 //ZnztB7mkDB+cDq9aEYIFR0NnUT8d1SlPR0K/S42Ygy/byP96PjFV+Fkcs7kUA3 DlWUgJxC7hr5VnF/PxU11ydiU90NT6NmhRSz/0++tlgGQzNXnHuGMyTtvvHJj30= =BX1H -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 19 Mar 2014, Phillip Susi wrote: > Date: Wed, 19 Mar 2014 12:04:10 -0400 > From: Phillip Susi <psusi@ubuntu.com> > To: Lukáš Czerner <lczerner@redhat.com> > Cc: Andreas Dilger <adilger@dilger.ca>, linux-ext4@vger.kernel.org > Subject: Re: [PATCH] mke2fs: don't interact with a non tty > > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 3/19/2014 11:16 AM, Lukáš Czerner wrote: > > There was a discussion between me and Dave Chinner a while ago > > about exactly this topic and he made some very good points. You as > > a guy working with file system, or file system utilities, testing > > them and so on will probably see a ton of the warning like that. > > But like it, or not you're *not* the typical user - you're a > > developer and there is a huge difference. > > Whether I'm a typical user or a developer does not matter. This is > unix, and commands are expected to do what they are told. If you must > do such silly things, at the very least you need to not break scripts > when doing it, which means if stdin is not a tty, don't assume you are > being run by an idiot who needs hand holding and do what you've been > told to. > > Scripts and other tools like say, gparted, should not have to figure > out every silly thing you might complain about and add a stupid > - --yes-i-really-meant-it flag to every invocation. You're wrong. It does matter, because the usage habits of those two groups are entirely different and certainly mke2fs is not a tool designed for developers. It is a safety net for already overwhelmed sysadmins. And I do not understand how this is breaking scripts - it has been like that for a long time. I agree that it is a bug that we're still asking question even though there is not a tty attached, but as I said the right thing to do in that case is to fail rather than blindly continue despite the checks. If you want the latter, you can use the 'force' Luke :) Thanks! -Lukas > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2.0.17 (MingW32) > Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ > > iQEcBAEBAgAGBQJTKb/6AAoJEI5FoCIzSKrwefcIAIHJtyiquS/Zrs4LbCSW1Zoe > 5QoLx1ROxqP/Y5CP9mMCcRfsOTXMtZWqf116/Hg5SgIbJ76FGn7peChfPAKw477W > HFPWsUmWgdkdVRFJdqny/5MVaOHGd/bD9mt7/8suEwSXX3eAp/gDegKR9Srm+etY > n7V/G/uHWQgUQDbs8GAAcpIMA4Y1zcARB19fdNMr1evhDORdN7PAFqxqbixz3gU6 > //ZnztB7mkDB+cDq9aEYIFR0NnUT8d1SlPR0K/S42Ygy/byP96PjFV+Fkcs7kUA3 > DlWUgJxC7hr5VnF/PxU11ydiU90NT6NmhRSz/0++tlgGQzNXnHuGMyTtvvHJj30= > =BX1H > -----END PGP SIGNATURE----- >
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 3/19/2014 1:05 PM, Lukáš Czerner wrote: > You're wrong. It does matter, because the usage habits of those > two groups are entirely different and certainly mke2fs is not a > tool designed for developers. It is designed for system administrators who are used to tools doing what they ask, and not second guessing them, even if that means doing something harmful. Unix lets you shoot yourself in the foot. There are hundreds of other similar "mistakes" an admin could make that would cause similar harm, and none of them stop and say are you really sure? Neither should mkfs. > It is a safety net for already overwhelmed sysadmins. And I do not > understand how this is breaking scripts - it has been like that > for a long time. It breaks scripts because the tool fails when it shouldn't, and the script now has to work around it with silly, fragile logic like "I'm being asked to make an ext[234] filesystem on a regular file, so I need to add this silly --i-meant-it flag." and "I'm, being asked to make a btrfs filesystem on this disk, so I better add this other silly flag to make sure it doesn't fail because there's already another fs there" and "filesystem x needs this third silly flag" and so on. > I agree that it is a bug that we're still asking question even > though there is not a tty attached, but as I said the right thing > to do in that case is to fail rather than blindly continue despite > the checks. If you want the latter, you can use the 'force' Luke > :) Scripts should not have to keep adding different - --yes-stupid-i-really-meant-it flags for special cased different filesystems to get what they already said they wanted. This is not the unix way. If you want hand holding, do it the way rm did: add a flag that turns the hand holding on, and set up an alias so you don't forget it when running it interactively. Or better yet, use an interactive gui tool that is designed to hold your hand and not be scripted. At the very least, don't assume the script is wrong because you can't ask "are you sure, dummy?". -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.17 (MingW32) Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBAgAGBQJTKdXmAAoJEI5FoCIzSKrw0R4H/R47g6bDEf749OH/BiRiNgLr k5n+4e0ep68DRkAAWLuXdAjr29LCZiFLAWd2XV69aQ/Ls3Nj+R8fKZ6v3IharRHg xDOFgrmv5QF7m3FgQVPS6qQEgCtGIprJdurQbcjwhQ26MwEF37hLRzJk4GRBLy7E zydUpZrVvzzsWPX4/k8bI3ESFZoMiuX9zEEd9Fl3hjHMByeZ+zqnLSJgSBt+y3eU fSpFY/F4pdzWKhEe0SOat1lGy6qFu92DVbRdJYTAGuDHiKqT7SHkpvt503+GHEAJ uo+jN4JU1FFSn5+3+fCsJb3XSmrIbbsXaIWBi8/VsujhCaHXnxG48gblNf9KCWg= =WhUQ -----END PGP SIGNATURE----- -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/misc/util.c b/misc/util.c index 92ab79f..c469744 100644 --- a/misc/util.c +++ b/misc/util.c @@ -71,6 +71,8 @@ void proceed_question(void) char buf[256]; const char *short_yes = _("yY"); + if (!isatty(0)) + return; fflush(stdout); fflush(stderr); fputs(_("Proceed anyway? (y,n) "), stdout);
mke2fs displays an interactive y/n prompt for certain situations. This is not appropriate to do when stdin is not a tty. Signed-off-by: Phillip Susi <psusi@ubuntu.com> --- misc/util.c | 2 ++ 1 file changed, 2 insertions(+)