On Tue, Mar 24, 2015 at 10:07:03AM -0400, Bobby Powers wrote: > Josh Poimboeuf wrote: > > Well, IMO, invalid user input isn't a good reason to pollute the printk > > buffer. Maybe you could write a simple wrapper script which gives the > > user a more helpful error message? > > Writing '1' to the enable file when it is already enabled isn't > invalid input, its an idempotent interface. The intention of the user > is to enable livepatching. We don't really know the intention of the user. What if they meant to echo 0 instead of 1? Or meant to write 1 to another file? Either way we'd have a silent failure with the proposed patch. IMHO we should always try to warn the user when they're trying to do something which doesn't make sense (especially for something as critical as live patching). > If it is already enabled, their request > isn't invalid, it just doesn't need any new action to be taken. If > userspace needs to enable (or disable) live patching, why force them > to check the previous value and test around each write (where now you > may race between read and subsequent write)? I don't think you need to worry too much about races. If the write returns EINVAL, then read it. If the read value is the same as the attempted write value, you can treat it as a success. Otherwise report an error (or retry). Yes, not very bash-friendly, but that's only if you want idempotent semantics. But the sysfs interface isn't really designed for end users anyway. There will probably be more sysfs complexity coming when we add a consistency model, so a cmdline tool is probably a good idea regardless. -- Josh -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
