| Message ID | 20210802214645.2633028-4-krisman@collabora.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Test the new fanotify FAN_FS_ERROR event | expand |
On Tue, Aug 3, 2021 at 12:47 AM Gabriel Krisman Bertazi <krisman@collabora.com> wrote: > > Verify the FID provided in the event. If the testcase has a null inode, > this is assumed to be a superblock error (i.e. null FH). > > Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com> > --- > .../kernel/syscalls/fanotify/fanotify20.c | 51 +++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c > index fd5cfb8744f1..d8d788ae685f 100644 > --- a/testcases/kernel/syscalls/fanotify/fanotify20.c > +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c > @@ -40,6 +40,14 @@ > > #define FAN_EVENT_INFO_TYPE_ERROR 4 > > +#ifndef FILEID_INVALID > +#define FILEID_INVALID 0xff > +#endif > + > +#ifndef FILEID_INO32_GEN > +#define FILEID_INO32_GEN 1 > +#endif > + > struct fanotify_event_info_error { > struct fanotify_event_info_header hdr; > __s32 error; > @@ -57,6 +65,9 @@ static const struct test_case { > char *name; > int error; > unsigned int error_count; > + > + /* inode can be null for superblock errors */ > + unsigned int *inode; Any reason not to use fanotify_fid_t * like fanotify16.c? Thanks, Amir.
Amir Goldstein <amir73il@gmail.com> writes: > On Tue, Aug 3, 2021 at 12:47 AM Gabriel Krisman Bertazi > <krisman@collabora.com> wrote: >> >> Verify the FID provided in the event. If the testcase has a null inode, >> this is assumed to be a superblock error (i.e. null FH). >> >> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com> >> --- >> .../kernel/syscalls/fanotify/fanotify20.c | 51 +++++++++++++++++++ >> 1 file changed, 51 insertions(+) >> >> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c >> index fd5cfb8744f1..d8d788ae685f 100644 >> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c >> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c >> @@ -40,6 +40,14 @@ >> >> #define FAN_EVENT_INFO_TYPE_ERROR 4 >> >> +#ifndef FILEID_INVALID >> +#define FILEID_INVALID 0xff >> +#endif >> + >> +#ifndef FILEID_INO32_GEN >> +#define FILEID_INO32_GEN 1 >> +#endif >> + >> struct fanotify_event_info_error { >> struct fanotify_event_info_header hdr; >> __s32 error; >> @@ -57,6 +65,9 @@ static const struct test_case { >> char *name; >> int error; >> unsigned int error_count; >> + >> + /* inode can be null for superblock errors */ >> + unsigned int *inode; > > Any reason not to use fanotify_fid_t * like fanotify16.c? No reason other than I didn't notice they existed. Sorry. I will get this fixed.
On Wed, Aug 4, 2021 at 7:54 AM Gabriel Krisman Bertazi <krisman@collabora.com> wrote: > > Amir Goldstein <amir73il@gmail.com> writes: > > > On Tue, Aug 3, 2021 at 12:47 AM Gabriel Krisman Bertazi > > <krisman@collabora.com> wrote: > >> > >> Verify the FID provided in the event. If the testcase has a null inode, > >> this is assumed to be a superblock error (i.e. null FH). > >> > >> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com> > >> --- > >> .../kernel/syscalls/fanotify/fanotify20.c | 51 +++++++++++++++++++ > >> 1 file changed, 51 insertions(+) > >> > >> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c > >> index fd5cfb8744f1..d8d788ae685f 100644 > >> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c > >> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c > >> @@ -40,6 +40,14 @@ > >> > >> #define FAN_EVENT_INFO_TYPE_ERROR 4 > >> > >> +#ifndef FILEID_INVALID > >> +#define FILEID_INVALID 0xff > >> +#endif > >> + > >> +#ifndef FILEID_INO32_GEN > >> +#define FILEID_INO32_GEN 1 > >> +#endif > >> + > >> struct fanotify_event_info_error { > >> struct fanotify_event_info_header hdr; > >> __s32 error; > >> @@ -57,6 +65,9 @@ static const struct test_case { > >> char *name; > >> int error; > >> unsigned int error_count; > >> + > >> + /* inode can be null for superblock errors */ > >> + unsigned int *inode; > > > > Any reason not to use fanotify_fid_t * like fanotify16.c? > > No reason other than I didn't notice they existed. Sorry. I will get > this fixed. No problem. That's what review is for ;-) BTW, unless anyone is specifically interested I don't think there is a reason to re post the test patches before the submission request. Certainly not for the small fixes that I requested. I do request that you post a link to a branch with the fixed test so that we can experiment with the kernel patches. I've also CC'ed Matthew who may want to help with review of the test and man page that you posted in the cover letter [1]. Thanks, Amir. [1] https://lore.kernel.org/linux-ext4/20210802214645.2633028-1-krisman@collabora.com/T/#m9cf637c6aca94e28390f61deac5a53afbc9e88ae
On Wed, Aug 04, 2021 at 08:39:55AM +0300, Amir Goldstein wrote: > On Wed, Aug 4, 2021 at 7:54 AM Gabriel Krisman Bertazi > <krisman@collabora.com> wrote: > > > > Amir Goldstein <amir73il@gmail.com> writes: > > > > > On Tue, Aug 3, 2021 at 12:47 AM Gabriel Krisman Bertazi > > > <krisman@collabora.com> wrote: > > >> > > >> Verify the FID provided in the event. If the testcase has a null inode, > > >> this is assumed to be a superblock error (i.e. null FH). > > >> > > >> Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com> > > >> --- > > >> .../kernel/syscalls/fanotify/fanotify20.c | 51 +++++++++++++++++++ > > >> 1 file changed, 51 insertions(+) > > >> > > >> diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c > > >> index fd5cfb8744f1..d8d788ae685f 100644 > > >> --- a/testcases/kernel/syscalls/fanotify/fanotify20.c > > >> +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c > > >> @@ -40,6 +40,14 @@ > > >> > > >> #define FAN_EVENT_INFO_TYPE_ERROR 4 > > >> > > >> +#ifndef FILEID_INVALID > > >> +#define FILEID_INVALID 0xff > > >> +#endif > > >> + > > >> +#ifndef FILEID_INO32_GEN > > >> +#define FILEID_INO32_GEN 1 > > >> +#endif > > >> + > > >> struct fanotify_event_info_error { > > >> struct fanotify_event_info_header hdr; > > >> __s32 error; > > >> @@ -57,6 +65,9 @@ static const struct test_case { > > >> char *name; > > >> int error; > > >> unsigned int error_count; > > >> + > > >> + /* inode can be null for superblock errors */ > > >> + unsigned int *inode; > > > > > > Any reason not to use fanotify_fid_t * like fanotify16.c? > > > > No reason other than I didn't notice they existed. Sorry. I will get > > this fixed. > > No problem. That's what review is for ;-) > > BTW, unless anyone is specifically interested I don't think there > is a reason to re post the test patches before the submission request. > Certainly not for the small fixes that I requested. > > I do request that you post a link to a branch with the fixed test > so that we can experiment with the kernel patches. > > I've also CC'ed Matthew who may want to help with review of the test > and man page that you posted in the cover letter [1]. I'll get around to going through both the LTP and man-page series by the end of this week. Feel free to also loop me in directly on any subsequent iterations of the like. /M
Hi all, > No problem. That's what review is for ;-) > BTW, unless anyone is specifically interested I don't think there > is a reason to re post the test patches before the submission request. > Certainly not for the small fixes that I requested. > I do request that you post a link to a branch with the fixed test > so that we can experiment with the kernel patches. > I've also CC'ed Matthew who may want to help with review of the test > and man page that you posted in the cover letter [1]. @Amir Thanks a lot for your review, agree with all you mentioned. @Gabriel Thanks for your contribution. I'd also consider squashing some of the commits. Kind regards, Petr > Thanks, > Amir. > [1] https://lore.kernel.org/linux-ext4/20210802214645.2633028-1-krisman@collabora.com/T/#m9cf637c6aca94e28390f61deac5a53afbc9e88ae
Hey Gabriel, On Fri, Aug 20, 2021 at 12:21:19PM +0200, Petr Vorel wrote: > Hi all, > > > No problem. That's what review is for ;-) > > > BTW, unless anyone is specifically interested I don't think there > > is a reason to re post the test patches before the submission request. > > Certainly not for the small fixes that I requested. > > > I do request that you post a link to a branch with the fixed test > > so that we can experiment with the kernel patches. > > > I've also CC'ed Matthew who may want to help with review of the test > > and man page that you posted in the cover letter [1]. > > @Amir Thanks a lot for your review, agree with all you mentioned. > > @Gabriel Thanks for your contribution. I'd also consider squashing some of the > commits. Is the FAN_FS_ERROR feature to be included within the 5.15 release? If so, I may need to do some shuffling around as these LTP tests collide with the ones I author for the FAN_REPORT_PIDFD series. /M
On Sat 21-08-21 07:58:07, Matthew Bobrowski wrote: > On Fri, Aug 20, 2021 at 12:21:19PM +0200, Petr Vorel wrote: > > Hi all, > > > > > No problem. That's what review is for ;-) > > > > > BTW, unless anyone is specifically interested I don't think there > > > is a reason to re post the test patches before the submission request. > > > Certainly not for the small fixes that I requested. > > > > > I do request that you post a link to a branch with the fixed test > > > so that we can experiment with the kernel patches. > > > > > I've also CC'ed Matthew who may want to help with review of the test > > > and man page that you posted in the cover letter [1]. > > > > @Amir Thanks a lot for your review, agree with all you mentioned. > > > > @Gabriel Thanks for your contribution. I'd also consider squashing some of the > > commits. > > Is the FAN_FS_ERROR feature to be included within the 5.15 release? If so, > I may need to do some shuffling around as these LTP tests collide with the > ones I author for the FAN_REPORT_PIDFD series. No, I don't think FAN_FS_ERROR is quite ready for the coming merge window. So you should be fine. Honza
On Mon, Aug 23, 2021 at 11:35:24AM +0200, Jan Kara wrote: > On Sat 21-08-21 07:58:07, Matthew Bobrowski wrote: > > On Fri, Aug 20, 2021 at 12:21:19PM +0200, Petr Vorel wrote: > > > Hi all, > > > > > > > No problem. That's what review is for ;-) > > > > > > > BTW, unless anyone is specifically interested I don't think there > > > > is a reason to re post the test patches before the submission request. > > > > Certainly not for the small fixes that I requested. > > > > > > > I do request that you post a link to a branch with the fixed test > > > > so that we can experiment with the kernel patches. > > > > > > > I've also CC'ed Matthew who may want to help with review of the test > > > > and man page that you posted in the cover letter [1]. > > > > > > @Amir Thanks a lot for your review, agree with all you mentioned. > > > > > > @Gabriel Thanks for your contribution. I'd also consider squashing some of the > > > commits. > > > > Is the FAN_FS_ERROR feature to be included within the 5.15 release? If so, > > I may need to do some shuffling around as these LTP tests collide with the > > ones I author for the FAN_REPORT_PIDFD series. > > No, I don't think FAN_FS_ERROR is quite ready for the coming merge window. > So you should be fine. Alrighty, thanks for letting me know Jan. /M
Matthew Bobrowski <repnop@google.com> writes: > Hey Gabriel, > > On Fri, Aug 20, 2021 at 12:21:19PM +0200, Petr Vorel wrote: >> Hi all, >> >> > No problem. That's what review is for ;-) >> >> > BTW, unless anyone is specifically interested I don't think there >> > is a reason to re post the test patches before the submission request. >> > Certainly not for the small fixes that I requested. >> >> > I do request that you post a link to a branch with the fixed test >> > so that we can experiment with the kernel patches. >> >> > I've also CC'ed Matthew who may want to help with review of the test >> > and man page that you posted in the cover letter [1]. >> >> @Amir Thanks a lot for your review, agree with all you mentioned. >> >> @Gabriel Thanks for your contribution. I'd also consider squashing some of the >> commits. > > Is the FAN_FS_ERROR feature to be included within the 5.15 release? If so, > I may need to do some shuffling around as these LTP tests collide with the > ones I author for the FAN_REPORT_PIDFD series. > Matthew, Hi, sorry for the delay. I took a short vacation and couldn't follow up. I think it is too late for 5.15, please go ahead with FAN_REPORT_PIDFD series and I will consider them in my future submission. Thank you,
diff --git a/testcases/kernel/syscalls/fanotify/fanotify20.c b/testcases/kernel/syscalls/fanotify/fanotify20.c index fd5cfb8744f1..d8d788ae685f 100644 --- a/testcases/kernel/syscalls/fanotify/fanotify20.c +++ b/testcases/kernel/syscalls/fanotify/fanotify20.c @@ -40,6 +40,14 @@ #define FAN_EVENT_INFO_TYPE_ERROR 4 +#ifndef FILEID_INVALID +#define FILEID_INVALID 0xff +#endif + +#ifndef FILEID_INO32_GEN +#define FILEID_INO32_GEN 1 +#endif + struct fanotify_event_info_error { struct fanotify_event_info_header hdr; __s32 error; @@ -57,6 +65,9 @@ static const struct test_case { char *name; int error; unsigned int error_count; + + /* inode can be null for superblock errors */ + unsigned int *inode; void (*trigger_error)(void); void (*prepare_fs)(void); } testcases[] = { @@ -83,6 +94,42 @@ struct fanotify_event_info_header *get_event_info( ((struct fanotify_event_info_error *) \ get_event_info((event), FAN_EVENT_INFO_TYPE_ERROR)) +#define get_event_info_fid(event) \ + ((struct fanotify_event_info_fid *) \ + get_event_info((event), FAN_EVENT_INFO_TYPE_FID)) + +int check_error_event_info_fid(struct fanotify_event_info_fid *fid, + const struct test_case *ex) +{ + int fail = 0; + struct file_handle *fh = (struct file_handle *) &fid->handle; + + if (!ex->inode) { + uint32_t *h = (uint32_t *) fh->f_handle; + + if (!(fh->handle_type == FILEID_INVALID && !h[0] && !h[1])) { + tst_res(TFAIL, "%s: file handle should have been invalid", + ex->name); + fail++; + } + return fail; + } else if (fh->handle_type == FILEID_INO32_GEN) { + uint32_t *h = (uint32_t *) fh->f_handle; + + if (h[0] != *ex->inode) { + tst_res(TFAIL, + "%s: Unexpected file handle inode (%u!=%u)", + ex->name, *ex->inode, h[0]); + fail++; + } + } else { + tst_res(TFAIL, "%s: Test can't handle received FH type (%d)", + ex->name, fh->handle_type); + } + + return fail; +} + int check_error_event_info_error(struct fanotify_event_info_error *info_error, const struct test_case *ex) { @@ -126,6 +173,7 @@ void check_event(char *buf, size_t len, const struct test_case *ex) struct fanotify_event_metadata *event = (struct fanotify_event_metadata *) buf; struct fanotify_event_info_error *info_error; + struct fanotify_event_info_fid *info_fid; int fail = 0; if (len < FAN_EVENT_METADATA_LEN) @@ -137,6 +185,9 @@ void check_event(char *buf, size_t len, const struct test_case *ex) info_error = get_event_info_error(event); fail += info_error ? check_error_event_info_error(info_error, ex) : 1; + info_fid = get_event_info_fid(event); + fail += info_fid ? check_error_event_info_fid(info_fid, ex) : 1; + if (!fail) tst_res(TPASS, "Successfully received: %s", ex->name); }
Verify the FID provided in the event. If the testcase has a null inode, this is assumed to be a superblock error (i.e. null FH). Signed-off-by: Gabriel Krisman Bertazi <krisman@collabora.com> --- .../kernel/syscalls/fanotify/fanotify20.c | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+)