On Wed, Apr 12, 2017 at 07:06:33AM -0400, Brian Foster wrote: > On Tue, Apr 11, 2017 at 04:43:26PM -0700, Darrick J. Wong wrote: > > On Wed, Apr 12, 2017 at 08:34:05AM +1000, Dave Chinner wrote: > > > On Tue, Apr 11, 2017 at 04:12:37PM +0200, Jan Tulak wrote: > > > > A dirty log in an obfuscated dump means that a corruption can happen > > > > when replaying the log (which contains unobfuscated data). Warn the user > > > > about this possibility. > > > > > > > The xlog workaround is copy&paste solution from repair/phase2.c and > > > > other tools, because the function is not implemented in libxlog. > > > > > > > > Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx> > > > > > > I think this is overkill. mdrestore is not the place > > > to be interpreting the state of the dumped image - it is a basic > > > "restore the image" program, not a "check the validity of the image" > > > program. > > > > > > Secondly, if people are having problems with running log recovery on > > > a restored obfuscated image and getting corruption and not knowing > > > why or what to do, then that is a /documentation and training/ > > > problem, not a code problem. > > > > > > i.e. the problem is that people who aren't developers are trying to > > > use tools that were written for developers to do forensic analysis > > > of failures. Don't dumb down the tool for clueless users - point the > > > users at the documentation that the tool requires to use correctly... > > > > Looking at the patch, that's a lot of code to add to mdrestore that has > > nothing to do with metadump restoration. For that matter, who's to say > > that the metadump'd image is even an XFS filesystem, and not just some > > garbage with the just the right superblock values to pass the > > perform_restore() checks? (Ok, ok, that was a little over the top.) > > > > Agreed wrt to the mdrestore bits... > > > The key change we're trying to make is to prevent people incorrectly > > replaying an XFS with a dirty log when the fs image has been restored > > from an obfuscated metadump. > > > > So in my mind this brings up two questions: First, how do we prevent > > log replay in such situations? Second, how do we teach people not to > > attempt log replay? As you point out, it's better that we educate > > people as what problems each tool tries to solve and where the sharp > > edges might be on the debugging tools, but the answer to the first > > question ensures that us fallible developers can't do something stupid > > even though we theoretically know better. > > > > Frankly, if the goal is to nudge n00b members of support teams away from > > a behavior that won't help them towards starting their failure analysis, > > then then I think we ought to patch the log recovery code to detect an > > obfuscated fs image, complain to dmesg about someone making an illogical > > move, and then refuse to mount the log. > > > > I don't think this is really appropriate. Some users may very well have > no other option but to create a dirty log + obfuscated metadump for > whatever security/privacy reasons they have. The purpose of warning in > that case is to notify the user to either verify the resulting image > shows whatever problems are exhibited by the original fs and no others, > or to notify the developer that other corruption might exist and to > ignore it as a side effect of the metadump process itself (provided it > doesn't interfere with rca of the original problem). Refusing to run log > recovery in such cases just gets in the way. > > I'm not tied to having an mdrestore warning at all, but I'd much prefer > to see it there rather than include obfuscation logic in the kernel just > to facilitate a userspace tool to continue on silently corrupting > filesystem images. <nod> I've changed my mind overnight. Now I agree that we could put a message in at metadump time, because it's not too late to ask the user to try to send us a metadump w/ clean log. Eric also convinced me that it's not so trivial to detect an obfuscated image, so that simply won't work without a bunch of hackery. --D > > Brian > > > I'd rather push back on the incorrect behavior at the time it is done, > > instead of training people to ignore a priori warning messages. > > > > --D > > > > > Cheers, > > > > > > Dave. > > > -- > > > Dave Chinner > > > david@xxxxxxxxxxxxx > > > -- > > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > the body of a message to majordomo@xxxxxxxxxxxxxxx > > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
