On Fri, Nov 24, 2017 at 12:38 PM, Karel Zak <kzak@xxxxxxxxxx> wrote:
> On Fri, Nov 24, 2017 at 10:04:33AM +0200, Amir Goldstein wrote:
>> [cc: Karel Zak]
>>
>> On Fri, Nov 24, 2017 at 7:01 AM, Eryu Guan <eguan@xxxxxxxxxx> wrote:
>> > util-linux commit 6dede2f2f7c5 ("libmount: support MS_RDONLY on
>> > write-protected devices") changed the error message on read-only
>> > block device, and in the failure case printed one line message
>> > instead of two (for details please see comments in common/filter),
>> > and this change broke generic/050 and overlay/035.
>> >
>> > Fix it by adding more filter rules to _filter_ro_mount and updating
>> > associated .out files to unify the output from both old and new
>> > util-linux versions.
>> >
>> > Signed-off-by: Eryu Guan <eguan@xxxxxxxxxx>
>> > ---
>> > v4 (also passed with both old & new util-linux):
>> > - add more comments to explain the output differences between util-linux
>> > versions
>> > - print out message directly instead of using perl variables
>> > - add word "device" to failed ro mount message
>>
>>
>> Eryu,
>>
>> This looks good and you can take it as an ACK on the series.
>>
>> I've CC'ed Karel, so maybe he thinks about us fstests guys before making
>> these sort of changes again...
>
> Frankly, we have never promised that things like warning messages (or
> another messages) are stable interface. It's fragile to depend on this
> stuff...
>
I know. No complains. Just wanted to inform you of the downstream
ripples.
>> Thinking out loud, does xfstest even need to use mount program from
>> util-linux? Do we ever need anything other than the bare libc mount(2)?
>> We need it for -o loop, but that is the exception to the rule.
>
> Well, I don't think that create a parallel universe is the best
> solution.
>
I agree. However, xfstest uses 'mount' is a very particular way:
- User has to provide both block device and mount point
- xfstests already checks that those block device and mount point
are not in use in /proc/mounts *before* calling 'mount'
So I prefer the 'machine output format' solution going forward, but
since xfstests will need to continue supporting old util-linux installations
it might be less clumsy to use a simple C wrapper to mount(2), then
to carry these error format filters.
xfstests can also check for availability of the new machine format in
'mount' and if it doesn't exist fall back to using its private mount helper.
>> For fs that have mount helpers (cifs,nfs), we could use mount.$FSTYP
>> directly (what error formats are reported from the helpers??). For all other
>> fs we can write a simple t_mount program to wrap libc mount(2) and not be
>> dependent on util-linux error message formats.
>> Maybe we can consider it next time util-linux changes..
>>
>> Another idea to through in the air in the direction of Karel -
>> Maybe it makes sense for util-linux to check some env variable
>> and then print all error messages in a unified machine format, e.g.:
>> fprintf("%s: errno=%d\n", progname, errno);
>
> This is good idea, I can try to implement it into libmount.
>
That would be great.
Thanks,
Amir.
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
