Messages in this thread | | | Date | Fri, 22 May 2015 15:34:36 +0200 | From | Marcin Jabrzyk <> | Subject | Re: [PATCH] zram: check compressor name before setting it |
| |
Hello Minchan,
On 22/05/15 15:14, Minchan Kim wrote: > Hello Sergey, > > On Fri, May 22, 2015 at 09:44:11PM +0900, Sergey Senozhatsky wrote: >> On (05/22/15 11:12), Marcin Jabrzyk wrote: >>>> >>>> no. >>>> >>>> zram already complains about failed comp backend creation. >>>> it's in dmesg (or syslog, etc.): >>>> >>>> "zram: Cannot initialise %s compressing backend" >>>> >>> OK, now I see that. Sorry for the noise. >>> >>>> second, there is not much value in exposing zcomp internals, >>>> especially when the result is just another line in dmesg output. >>> >>> From the other hand, the only valid values that can be written are >>> in 'comp_algorithm'. >>> So when writing other one, returning -EINVAL seems to be reasonable. >>> The user would get immediately information that he can't do that, >>> now the information can be very deferred in time. >> >> it's not. >> the error message appears in syslog right before we return -EINVAL >> back to user. > > Although Marcin's description is rather misleading, I like the patch. > Every admin doesn't watch dmesg output. Even people could change loglevel > simply so KERN_INFO would be void in that case. Sorry for being confusing, at the first time I've overlooked that error message in syslog. I didn't thought about looking for handling exactly this error in completely different place.
> > Instant error propagation is more strighforward for user point of view > rather than delaying with depending on another event.
Yes this was my exact motivation. Instant value can be detected in scripts etc. Easier to debug in automated environment.
> > Thanks. > >> >> -ss >> >>> I'm not for exposing more internals, but getting -EINVAL would be nice I >
If this would be ok, I can prepare v2 with better description and with less exposing zcomp internals.
Best regards, Marcin Jabrzyk
| |