| Message ID | 20220311095101.10112-1-chrubis@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | pty/pty07: Restore active console after the testrun | expand |
Hi, On 11. 03. 22 10:51, Cyril Hrubis wrote: > The test, as a side effect, switches to a different console during the > run, which may confuse both users and automated test systems. > > Fix that by saving the console active at the start of the test and > restore it in the test cleanup. > > Signed-off-by: Cyril Hrubis <chrubis@suse.cz> > --- > testcases/kernel/pty/pty07.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/testcases/kernel/pty/pty07.c b/testcases/kernel/pty/pty07.c > index 462569c4a..af2dccb32 100644 > --- a/testcases/kernel/pty/pty07.c > +++ b/testcases/kernel/pty/pty07.c > @@ -40,6 +40,8 @@ static int test_tty_port = 8; > static int fd = -1; > static struct tst_fzsync_pair fzp; > > +static unsigned short vt_active; > + > static void *open_close(void *unused) > { > int i; > @@ -76,16 +78,27 @@ static void do_test(void) > > static void setup(void) > { > + struct vt_stat stat; > + > sprintf(tty_path, "/dev/tty%d", test_tty_port); > fd = SAFE_OPEN(tty_path, O_RDWR); > + SAFE_IOCTL(fd, VT_GETSTATE, &stat); > + vt_active = stat.v_active; > + > + tst_res(TINFO, "Saving active console %i", vt_active); > + > tst_fzsync_pair_init(&fzp); > } > > static void cleanup(void) > { > - tst_fzsync_pair_cleanup(&fzp); > - if (fd >= 0) > + if (fd >= 0) { > + tst_res(TINFO, "Restoring active console"); > + SAFE_IOCTL(fd, VT_ACTIVATE, vt_active); > SAFE_CLOSE(fd); > + } > + > + tst_fzsync_pair_cleanup(&fzp); If you move the fzsync cleanup to the end of cleanup(), you can end up with the open_close() thread racing against fd cleanup. > } > > static struct tst_test test = {
Hi! > > static void cleanup(void) > > { > > - tst_fzsync_pair_cleanup(&fzp); > > - if (fd >= 0) > > + if (fd >= 0) { > > + tst_res(TINFO, "Restoring active console"); > > + SAFE_IOCTL(fd, VT_ACTIVATE, vt_active); > > SAFE_CLOSE(fd); > > + } > > + > > + tst_fzsync_pair_cleanup(&fzp); > > If you move the fzsync cleanup to the end of cleanup(), you can end up > with the open_close() thread racing against fd cleanup. Ah, right, looking closely at the fzsync, the thread B may be racing against the restoration in the case that something caused premature exit in the thread A. Will move the call back to the start of the cleanup().
Hello, Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> > static void cleanup(void) >> > { >> > - tst_fzsync_pair_cleanup(&fzp); >> > - if (fd >= 0) >> > + if (fd >= 0) { >> > + tst_res(TINFO, "Restoring active console"); >> > + SAFE_IOCTL(fd, VT_ACTIVATE, vt_active); >> > SAFE_CLOSE(fd); >> > + } >> > + >> > + tst_fzsync_pair_cleanup(&fzp); >> >> If you move the fzsync cleanup to the end of cleanup(), you can end up >> with the open_close() thread racing against fd cleanup. > > Ah, right, looking closely at the fzsync, the thread B may be racing > against the restoration in the case that something caused premature exit > in the thread A. > > Will move the call back to the start of the cleanup(). > > -- > Cyril Hrubis > chrubis@suse.cz Why are we using /dev/tty8 instead of allocating a pty with /dev/ptmx?
Hi! > > Ah, right, looking closely at the fzsync, the thread B may be racing > > against the restoration in the case that something caused premature exit > > in the thread A. > > > > Will move the call back to the start of the cleanup(). > > > > -- > > Cyril Hrubis > > chrubis@suse.cz > > Why are we using /dev/tty8 instead of allocating a pty with /dev/ptmx? I do not think that we can VT_ACTIVATE on pseudoterminals, as far as I can tell we have to have real console for that.
Cyril Hrubis <chrubis@suse.cz> writes: > Hi! >> > Ah, right, looking closely at the fzsync, the thread B may be racing >> > against the restoration in the case that something caused premature exit >> > in the thread A. >> > >> > Will move the call back to the start of the cleanup(). >> > >> > -- >> > Cyril Hrubis >> > chrubis@suse.cz >> >> Why are we using /dev/tty8 instead of allocating a pty with /dev/ptmx? > > I do not think that we can VT_ACTIVATE on pseudoterminals, as far as I > can tell we have to have real console for that. OK, this looks good then. Reviewed-by: Richard Palethorpe <rpalethorpe@suse.com>
Hi! Applied with the change requested by Martin.
diff --git a/testcases/kernel/pty/pty07.c b/testcases/kernel/pty/pty07.c index 462569c4a..af2dccb32 100644 --- a/testcases/kernel/pty/pty07.c +++ b/testcases/kernel/pty/pty07.c @@ -40,6 +40,8 @@ static int test_tty_port = 8; static int fd = -1; static struct tst_fzsync_pair fzp; +static unsigned short vt_active; + static void *open_close(void *unused) { int i; @@ -76,16 +78,27 @@ static void do_test(void) static void setup(void) { + struct vt_stat stat; + sprintf(tty_path, "/dev/tty%d", test_tty_port); fd = SAFE_OPEN(tty_path, O_RDWR); + SAFE_IOCTL(fd, VT_GETSTATE, &stat); + vt_active = stat.v_active; + + tst_res(TINFO, "Saving active console %i", vt_active); + tst_fzsync_pair_init(&fzp); } static void cleanup(void) { - tst_fzsync_pair_cleanup(&fzp); - if (fd >= 0) + if (fd >= 0) { + tst_res(TINFO, "Restoring active console"); + SAFE_IOCTL(fd, VT_ACTIVATE, vt_active); SAFE_CLOSE(fd); + } + + tst_fzsync_pair_cleanup(&fzp); } static struct tst_test test = {
The test, as a side effect, switches to a different console during the run, which may confuse both users and automated test systems. Fix that by saving the console active at the start of the test and restore it in the test cleanup. Signed-off-by: Cyril Hrubis <chrubis@suse.cz> --- testcases/kernel/pty/pty07.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)