Lennart Poettering <[email protected]> writes: > On Wed, 20.05.15 10:37, [email protected] ([email protected]) wrote: > >> From: Jan Synacek <[email protected]> >> >> Allow certain configuration options to be specified as percentages. For >> example, in journald.conf, SystemMaxUse= can now also be specified as 33%. >> >> There is a slight "problem" with the patch. It parses option names to >> determine >> what filesystem it should use to get the available space from. This approach >> is probably not the rigth thing to do, but I couldn't think of a better one. >> Another approach that came to my mind was to use the highest bit of the off_t >> value returned by the parser to indicate if the value was a percentage, or >> a normal value. This would require to rewrite a significant amount of code >> which now counts on the values being definite (not percentages), and would, >> IMHO, be hardly any improvement at all. >> >> What do you think? Is there a better way to implement this functionality? Is >> it >> a real problem to parse the option lvalues like that? > > Yes, it's ugly! Also, it's problematic since disk sizes and space > change dynamically, hence evaluating this only when parsing is not > enough. > > What about this: introduce a new type: > > typedef struct SizeParameter { > uint64_t value; > bool relative; > } SizeParameter; > > When .relative is false, then .value is an absolute value, otherwise > it's a relative value normalized to let's say 0x100000000 (so that > this value equals 100%, and values below it < 100% and above it > > 100%).
Would you mind explaining this a bit more? I'm not sure if I understand this correctly, especially the "< 100%" and "> %100" parts. It doesn't seem to be needed at all. You always need the info from statvfs anyway, if the value is a percentage. If not, the value can be used as-is. > Then add new helper calls: > > int size_parameter_from_string(const char *s, SizeParameter *ret); > uint64 size_parameter_evaluate(SizeParameter *p, uint64_t base); > > > The latter should return .value as-is if p->relative is false, and > (base * .value) >> 32 otherwise. Why is "base" needed here? > THen, change the appropriate places to use SizeParameter instead of > simple uint64_t where necessary, and use size_parameter_evaluate() > with the data from statvfs when the actual value is required. Maybe I totally misunderstood your suggestion:) However, I tried to implement something similar and it turned out to be non-trivial, since there are quite a few places that need to be rewritten and tested, plus there is some duplicate code in coredump.c that needs the testing as well, so it may take a while. > Lennart Cheers, -- Jan Synacek Software Engineer, Red Hat
signature.asc
Description: PGP signature
_______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
