On 12/7/16 7:27 AM, Jan Tulak wrote: > Hi guys, ... > Number one is simple: What values can use block/sector sizes as user input? > There is an inconsistency or ambiguity between manual page and the code. Look > at man page for -d agsize. > > agsize=value > This is an alternative to using the agcount subop‐ > tion. The value is the desired size of the alloca‐ > tion group expressed in bytes (usually using the m > ^^^^^^^^ > or g suffixes). This value must be a multiple of > [ ... ] > Ok, 'm' and 'g' are just given as examples here, not an exhaustive list. > The option -d agsize explicitly states that it accepts size in bytes, in a > similar tone to the one used for describe allowed values for -s/-b size: > > value in bytes with size=value > ^^^^^^^^ > > However, -d agsize=1234s input was accepted as valid until now. Is the manual > page misleading, or are the options where b/s suffix is forbidden are > block/sector size definitions? I decided to err on the compatibility side and > kept the current behaviour - only blocksize or sectorsize can't be stated in > blocks and sectors, but it can be easily changed. > > I will send an update for xfstests once I know what behaviour is correct. cvtnum() handles 'b' and 's' along with all the other more normal logarithmic modifiers (k, m, g, t, p, e) so I think there's no real reason to exclude 'b' or 's' from options that take bytes. cvtnum() today already makes sure that the appropriate multiplier unit (for b or s) is available at the time of the call, so that all seems ok code-wise. (Honestly, while saying "-s size=512 -b size=8s" seems a bit weird, it's not wrong, so I see no reason to exclude it. Of course the sector size cannot be specified in sector units... :) ) I see no problem with allowing s and b as units for any option as long as the underlying unit is available by the time it's processed. > > The other question about this patchset is: As we are saving all the values in > the opt_params table, and the values have different types, I thought it > necessary to not use a single data type for everything and created an union > field (could be easily changed to struct, that would not change anything > important). Do you see any non-adressed issue with this approach? Is there > another way how to solve the problem? I think it'll take a read through the patches to properly answer this one :) -Eric -- 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
