pidns31: fix EACCESS error in mq_open()

pidns31: mq_open() fails with EACCESS.

pidns31 test case fails with EACCESS - due to the leading '/' in mqname. See comments in the patch below for details.

Signed-off-by Sukadev Bhattiprolu <[email protected]>

---
Index: testcases/kernel/containers/pidns/pidns31.c
===================================================================
RCS file: /cvsroot/ltp/ltp/testcases/kernel/containers/pidns/pidns31.c,v retrieving revision 1.6
diff -u -p -r1.6 pidns31.c

kernel/containers/pidns/pidns31.c 4 Aug 2010 23:24:54 -0000 renamed from kernel/containers/pidns/pidns31.c 14 Nov 2009 23:58:26 -0000      1.6 +8 -1

@@ -61,7 +61,14 @@
  char *TCID = "pidns31";
  int TST_TOTAL = 1;
 
-char *mqname = "/mq1";
+
+/*
+ * NOTE: The mq_open() interface in glibc requires mqname to start with a '/'
+ *      and the glibc call skips the leading '/' when invoking the system
+ *      call. Since we bypass the glibc mq_open() (and use syscall()) we
+ *      define mqname without the leading '/'.
+ */
+char *mqname = "mq1";
  int result = TFAIL;
 
  int errno;

Garrett Cooper [[email protected]] wrote:

| On Wed, Aug 4, 2010 at 4:52 PM, Sukadev Bhattiprolu
| <[email protected]> wrote:
| >
| > pidns31: mq_open() fails with EACCESS.
| >
| > pidns31 test case fails with EACCESS - due to the leading '/' in
| > mqname. See comments in the patch below for details.
| >
| > Signed-off-by: Sukadev Bhattiprolu <[email protected]>
| > ---
| > Index: testcases/kernel/containers/pidns/pidns31.c
| > ===================================================================
| > RCS file: /cvsroot/ltp/ltp/testcases/kernel/containers/pidns/pidns31.c,v
| > retrieving revision 1.6
| > diff -u -p -r1.6 pidns31.c
| > --- testcases/kernel/containers/pidns/pidns31.c 14 Nov 2009 23:58:26 -0000      1.6
| > +++ testcases/kernel/containers/pidns/pidns31.c 4 Aug 2010 23:24:54 -0000
| > @@ -61,7 +61,14 @@
| >  char *TCID = "pidns31";
| >  int TST_TOTAL = 1;
| >
| > -char *mqname = "/mq1";
| > +
| > +/*
| > + * NOTE: The mq_open() interface in glibc requires mqname to start with a '/'
| > + *      and the glibc call skips the leading '/' when invoking the system
| > + *      call. Since we bypass the glibc mq_open() (and use syscall()) we
| > + *      define mqname without the leading '/'.
| > + */
| > +char *mqname = "mq1";
| >  int result = TFAIL;
| >
| >  int errno;
| > @@ -264,6 +271,7 @@ int main(int argc, char *argv[])
| >        }
| >
| >        syscall(__NR_mq_unlink, mqname);
| > +
| >        mqd = syscall(__NR_mq_open, mqname, O_RDWR|O_CREAT|O_EXCL, 0777, NULL);
| >        if (mqd == (mqd_t)-1) {
| >                tst_resm(TBROK, "parent: mq_open() failed (%s)",
| 
| Did Linux become non-POSIX compliant :) (from
| <http://www.opengroup.org/onlinepubs/000095399/functions/mq_open.html>)?

Heh. Good question but I think the mq_open() from glibc is compliant with this. I am just not sure if the call via syscall() needs to be compliant.

But if that is a concern, we need a separate a test case for that - pidns31 is using mq_* to test an unrelated feature. I think would make sense to apply this fix, allowing LTP to test the feature even on older distros.

Or pidns31 can go back to using the library interface and we could skip the test on old distros.

Sukadev.

Quoted 55 lines · Sukadev Bhattiprolu

Garrett Cooper [[email protected]] wrote:

| On Wed, Aug 4, 2010 at 4:52 PM, Sukadev Bhattiprolu
| <[email protected]> wrote:
| >
| > pidns31: mq_open() fails with EACCESS.
| >
| > pidns31 test case fails with EACCESS - due to the leading '/' in
| > mqname. See comments in the patch below for details.
| >
| > Signed-off-by: Sukadev Bhattiprolu <[email protected]>
| > ---
| > Index: testcases/kernel/containers/pidns/pidns31.c
| > ===================================================================
| > RCS file: /cvsroot/ltp/ltp/testcases/kernel/containers/pidns/pidns31.c,v
| > retrieving revision 1.6
| > diff -u -p -r1.6 pidns31.c
| > --- testcases/kernel/containers/pidns/pidns31.c 14 Nov 2009 23:58:26 -0000      1.6
| > +++ testcases/kernel/containers/pidns/pidns31.c 4 Aug 2010 23:24:54 -0000
| > @@ -61,7 +61,14 @@
| >  char *TCID = "pidns31";
| >  int TST_TOTAL = 1;
| >
| > -char *mqname = "/mq1";
| > +
| > +/*
| > + * NOTE: The mq_open() interface in glibc requires mqname to start with a '/'
| > + *      and the glibc call skips the leading '/' when invoking the system
| > + *      call. Since we bypass the glibc mq_open() (and use syscall()) we
| > + *      define mqname without the leading '/'.
| > + */
| > +char *mqname = "mq1";
| >  int result = TFAIL;
| >
| >  int errno;
| > @@ -264,6 +271,7 @@ int main(int argc, char *argv[])
| >        }
| >
| >        syscall(__NR_mq_unlink, mqname);
| > +
| >        mqd = syscall(__NR_mq_open, mqname, O_RDWR|O_CREAT|O_EXCL, 0777, NULL);
| >        if (mqd == (mqd_t)-1) {
| >                tst_resm(TBROK, "parent: mq_open() failed (%s)",
|
| Did Linux become non-POSIX compliant :) (from
| <http://www.opengroup.org/onlinepubs/000095399/functions/mq_open.html>)?

Heh. Good question but I think the mq_open() from glibc is compliant with this. I am just not sure if the call via syscall() needs to be compliant.

But if that is a concern, we need a separate a test case for that - pidns31 is using mq_* to test an unrelated feature. I think would make sense to apply this fix, allowing LTP to test the feature even on older distros.

Quoted 2 lines

Or pidns31 can go back to using the library interface and we could skip the test on old distros.

I would prefer this.

Quoted 57 lines · Subrata Modak

Quoted 55 lines · Sukadev Bhattiprolu

Garrett Cooper [[email protected]] wrote:

| On Wed, Aug 4, 2010 at 4:52 PM, Sukadev Bhattiprolu
| <[email protected]> wrote:
| >
| > pidns31: mq_open() fails with EACCESS.
| >
| > pidns31 test case fails with EACCESS - due to the leading '/' in
| > mqname. See comments in the patch below for details.
| >
| > Signed-off-by: Sukadev Bhattiprolu <[email protected]>
| > ---
| > Index: testcases/kernel/containers/pidns/pidns31.c
| > ===================================================================
| > RCS file: /cvsroot/ltp/ltp/testcases/kernel/containers/pidns/pidns31.c,v
| > retrieving revision 1.6
| > diff -u -p -r1.6 pidns31.c
| > --- testcases/kernel/containers/pidns/pidns31.c 14 Nov 2009 23:58:26 -0000      1.6
| > +++ testcases/kernel/containers/pidns/pidns31.c 4 Aug 2010 23:24:54 -0000
| > @@ -61,7 +61,14 @@
| >  char *TCID = "pidns31";
| >  int TST_TOTAL = 1;
| >
| > -char *mqname = "/mq1";
| > +
| > +/*
| > + * NOTE: The mq_open() interface in glibc requires mqname to start with a '/'
| > + *      and the glibc call skips the leading '/' when invoking the system
| > + *      call. Since we bypass the glibc mq_open() (and use syscall()) we
| > + *      define mqname without the leading '/'.
| > + */
| > +char *mqname = "mq1";
| >  int result = TFAIL;
| >
| >  int errno;
| > @@ -264,6 +271,7 @@ int main(int argc, char *argv[])
| >        }
| >
| >        syscall(__NR_mq_unlink, mqname);
| > +
| >        mqd = syscall(__NR_mq_open, mqname, O_RDWR|O_CREAT|O_EXCL, 0777, NULL);
| >        if (mqd == (mqd_t)-1) {
| >                tst_resm(TBROK, "parent: mq_open() failed (%s)",
|
| Did Linux become non-POSIX compliant :) (from
| <http://www.opengroup.org/onlinepubs/000095399/functions/mq_open.html>)?

Heh. Good question but I think the mq_open() from glibc is compliant with this. I am just not sure if the call via syscall() needs to be compliant.

But if that is a concern, we need a separate a test case for that - pidns31 is using mq_* to test an unrelated feature. I think would make sense to apply this fix, allowing LTP to test the feature even on older distros.

Quoted 4 lines

Quoted 2 lines

Or pidns31 can go back to using the library interface and we could skip the test on old distros.

I would prefer this.

Do I hear someone volunteering to add the required autoconf tests and tests for this :)?

Garrett Cooper [[email protected]] wrote:

| On Mon, Aug 16, 2010 at 7:52 AM, Subrata Modak
| <[email protected]> wrote:
| >
| > On Wed, 2010-08-04 at 18:43 -0700, Sukadev Bhattiprolu wrote:
| >> Garrett Cooper [[email protected]] wrote:
| >> | On Wed, Aug 4, 2010 at 4:52 PM, Sukadev Bhattiprolu
| >> | <[email protected]> wrote:
| >> | >
| >> | > pidns31: mq_open() fails with EACCESS.
| >> | >
| >> | > pidns31 test case fails with EACCESS - due to the leading '/' in
| >> | > mqname. See comments in the patch below for details.
| >> | >
| >> | > Signed-off-by: Sukadev Bhattiprolu <[email protected]>
| >> | > ---
| >> | > Index: testcases/kernel/containers/pidns/pidns31.c
| >> | > ===================================================================
| >> | > RCS file: /cvsroot/ltp/ltp/testcases/kernel/containers/pidns/pidns31.c,v
| >> | > retrieving revision 1.6
| >> | > diff -u -p -r1.6 pidns31.c
| >> | > --- testcases/kernel/containers/pidns/pidns31.c 14 Nov 2009 23:58:26 -0000      1.6
| >> | > +++ testcases/kernel/containers/pidns/pidns31.c 4 Aug 2010 23:24:54 -0000
| >> | > @@ -61,7 +61,14 @@
| >> | >  char *TCID = "pidns31";
| >> | >  int TST_TOTAL = 1;
| >> | >
| >> | > -char *mqname = "/mq1";
| >> | > +
| >> | > +/*
| >> | > + * NOTE: The mq_open() interface in glibc requires mqname to start with a '/'
| >> | > + *      and the glibc call skips the leading '/' when invoking the system
| >> | > + *      call. Since we bypass the glibc mq_open() (and use syscall()) we
| >> | > + *      define mqname without the leading '/'.
| >> | > + */
| >> | > +char *mqname = "mq1";
| >> | >  int result = TFAIL;
| >> | >
| >> | >  int errno;
| >> | > @@ -264,6 +271,7 @@ int main(int argc, char *argv[])
| >> | >        }
| >> | >
| >> | >        syscall(__NR_mq_unlink, mqname);
| >> | > +
| >> | >        mqd = syscall(__NR_mq_open, mqname, O_RDWR|O_CREAT|O_EXCL, 0777, NULL);
| >> | >        if (mqd == (mqd_t)-1) {
| >> | >                tst_resm(TBROK, "parent: mq_open() failed (%s)",
| >> |
| >> | Did Linux become non-POSIX compliant :) (from
| >> | <http://www.opengroup.org/onlinepubs/000095399/functions/mq_open.html>)?
| >>
| >> Heh. Good question but I think the mq_open() from glibc is compliant
| >> with this. I am just not sure if the call via syscall() needs to be
| >> compliant.
| >>
| >> But if that is a concern, we need a separate a test case for that -
| >> pidns31 is using mq_* to test an unrelated feature. I think would
| >> make sense to apply this fix, allowing LTP to test the feature even
| >> on older distros.
| >>
| >> Or pidns31 can go back to using the library interface and we could
| >> skip the test on old distros.
| >
| > I would prefer this.
| 
| Do I hear someone volunteering to add the required autoconf tests and
| tests for this :)?

Well, I thought we could add a simple autoconf test for the pidns - but then realized that mq_open() calls were changed for many other test cases too. The test cases include mqueue.h, which would not be present in old distros anyway.

So, I don't see the point in fixing this just for pidns tests. If we want to use the syscall() interface, why don't we just design the test to work with the syscall behavior - i.e just remove the '/' from the name ?

Sukadev

Quoted 73 lines · Sukadev Bhattiprolu

Garrett Cooper [[email protected]] wrote:

| On Mon, Aug 16, 2010 at 7:52 AM, Subrata Modak
| <[email protected]> wrote:
| >
| > On Wed, 2010-08-04 at 18:43 -0700, Sukadev Bhattiprolu wrote:
| >> Garrett Cooper [[email protected]] wrote:
| >> | On Wed, Aug 4, 2010 at 4:52 PM, Sukadev Bhattiprolu
| >> | <[email protected]> wrote:
| >> | >
| >> | > pidns31: mq_open() fails with EACCESS.
| >> | >
| >> | > pidns31 test case fails with EACCESS - due to the leading '/' in
| >> | > mqname. See comments in the patch below for details.
| >> | >
| >> | > Signed-off-by: Sukadev Bhattiprolu <[email protected]>
| >> | > ---
| >> | > Index: testcases/kernel/containers/pidns/pidns31.c
| >> | > ===================================================================
| >> | > RCS file: /cvsroot/ltp/ltp/testcases/kernel/containers/pidns/pidns31.c,v
| >> | > retrieving revision 1.6
| >> | > diff -u -p -r1.6 pidns31.c
| >> | > --- testcases/kernel/containers/pidns/pidns31.c 14 Nov 2009 23:58:26 -0000      1.6
| >> | > +++ testcases/kernel/containers/pidns/pidns31.c 4 Aug 2010 23:24:54 -0000
| >> | > @@ -61,7 +61,14 @@
| >> | >  char *TCID = "pidns31";
| >> | >  int TST_TOTAL = 1;
| >> | >
| >> | > -char *mqname = "/mq1";
| >> | > +
| >> | > +/*
| >> | > + * NOTE: The mq_open() interface in glibc requires mqname to start with a '/'
| >> | > + *      and the glibc call skips the leading '/' when invoking the system
| >> | > + *      call. Since we bypass the glibc mq_open() (and use syscall()) we
| >> | > + *      define mqname without the leading '/'.
| >> | > + */
| >> | > +char *mqname = "mq1";
| >> | >  int result = TFAIL;
| >> | >
| >> | >  int errno;
| >> | > @@ -264,6 +271,7 @@ int main(int argc, char *argv[])
| >> | >        }
| >> | >
| >> | >        syscall(__NR_mq_unlink, mqname);
| >> | > +
| >> | >        mqd = syscall(__NR_mq_open, mqname, O_RDWR|O_CREAT|O_EXCL, 0777, NULL);
| >> | >        if (mqd == (mqd_t)-1) {
| >> | >                tst_resm(TBROK, "parent: mq_open() failed (%s)",
| >> |
| >> | Did Linux become non-POSIX compliant :) (from
| >> | <http://www.opengroup.org/onlinepubs/000095399/functions/mq_open.html>)?
| >>
| >> Heh. Good question but I think the mq_open() from glibc is compliant
| >> with this. I am just not sure if the call via syscall() needs to be
| >> compliant.
| >>
| >> But if that is a concern, we need a separate a test case for that -
| >> pidns31 is using mq_* to test an unrelated feature. I think would
| >> make sense to apply this fix, allowing LTP to test the feature even
| >> on older distros.
| >>
| >> Or pidns31 can go back to using the library interface and we could
| >> skip the test on old distros.
| >
| > I would prefer this.
|
| Do I hear someone volunteering to add the required autoconf tests and
| tests for this :)?

Well, I thought we could add a simple autoconf test for the pidns - but then realized that mq_open() calls were changed for many other test cases too. The test cases include mqueue.h, which would not be present in old distros anyway.

Quoted 3 lines

So, I don't see the point in fixing this just for pidns tests. If we want to use the syscall() interface, why don't we just design the test to work with the syscall behavior - i.e  just remove the '/' from the name ?

Garret,

Your thoughts ?