Re: [LTP] [PATCH v2] fallocate04: another check if SEEK_HOLE is not supported

Zeng Linggang
2015-05-21T01:25:57Z

On Wed, 2015-05-20 at 07:32 -0400, Jan Stancek wrote:

[...]
> > > > How about put them before lseek(SEEK_HOLE) ?
> > >
> > > I think we should do the check on all kernels. lseek() can only check that
> > > the
> > > hole exists, but current check also verifies that hole has correct size and
> > > content.
> > >
> >
> > Right. It is the strict way we need to do. But it would make the test
> > complex.
>
> I think we already check it strictly, see line 172-177. This covers blocks 1,2,3.
> The patch you proposed _adds_ one more check only for zero-ed block 2.
> My objection was that this seems redundant, because we already have
> a check (line 172-177), which covers all 3 blocks. Am I missing something?
>

Oh, I see. I misunderstand your reply previous. Right, the patch is
redundant.
Thank you.

> > Now, "turn that warning into TINFO" looks simple but effective.
> > If nobody reject, I will send new patch like:
>
> Looks OK to me, though maybe it should TBROK on kernels > 3.1 even when
> errno is EINVAL.

OK. I will fix it at the same time.

Thanks.

Best regards,
Zeng

>
> Regards,
> Jan
>
> >
> > - tst_resm(TWARN | TERRNO, "lseek() doesn't support
> > SEEK_HOLE");
> > + if (tst_kvercmp(3, 1, 0) < 0)
> > + tst_resm(TINFO, "lseek() doesn't support SEEK_HOLE, "
> > + "this is expected for < 3.1 kernels");
> > + } else {
> > + tst_resm(TINFO, "found a hole at '%ld' offset", ret);
> > }
> > - tst_resm(TINFO, "found a hole at '%ld' offset", ret);
> >
> > Thank you.
> >
> > Best regards,
> > Zeng
> >
> >
> > > Regards,
> > > Jan
> > [...]
> >
> >