On 4/5/17 8:00 AM, Jan Tulak wrote: > On Thu, Mar 16, 2017 at 11:59 PM, Eric Sandeen <sandeen@xxxxxxxxxxx> wrote: >> The only downside I see to this is that we used to echo back the same >> form that was specified, i.e. - >> >> # mkfs.xfs -f -d size=4e fsfile >> size 4e specified for data subvolume is too large, maximum is 262144 blocks >> # mkfs.xfs -f -d size=4611686018427387904 fsfile >> size 4611686018427387904 specified for data subvolume is too large, maximum is 262144 blocks >> # mkfs.xfs -f -d size=1125899906842624b fsfile >> size 1125899906842624b specified for data subvolume is too large, maximum is 262144 blocks >> >> now we always get back the raw byte value: >> >> # mkfs/mkfs.xfs -f -d size=4e fsfile >> size 4611686018427387904 specified for data subvolume is too large, maximum is 262144 blocks >> >> Anything that fails the getnum() checks echo back the original >> form on the cmdline; this case is different because getnum() passes, >> but by the time we do value checking all we have is the bytes. Is >> that intentional/desirable? >> >> So it's a change, not necessarily incorrect or unacceptable, but >> I wanted to highlight it and check. It wouldn't be too hard to >> convert it right away, but keep the string around for error printing >> purposes, I suppose. >> > > It goes back into the original reporting style later in the set. I > tried to change the patch to avoid this temporary change, but the > issue is, I loose the old raw strings before I have access to the > necessary indexes for the option table to read the raw string from it, > or before a code shuffling brings the reporting closer to where the > raw string still exists. > > Given that it is only a temporary change of style and it seems to be > too deeply depending on other changes, I'm keeping it as a TODO, but > not a blocker. Maybe if the patch can be split and one of the parts > moved after other commits... Even simply noting in the changelog that it's temporary, and the reason for the change, would be helpful. It's a big enough patchset that it's hard for the reviewer to keep everything in mind across 20+ patches. ;) Thanks, -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
